feat(calendar): compact night hours + fix cross-account shared-calendar edit/delete - #1025
Open
bartfaizoli76 wants to merge 7 commits into
Open
bartfaizoli76 wants to merge 7 commits into
bartfaizoli76 wants to merge 7 commits into
Conversation
Adds an opt-in Calendar setting ("Compact night hours", off by default)
that shrinks the 22:00-06:00 band in the day and week time-grid views to
a much smaller per-hour height, so a full 24h day fits with noticeably
less scrolling. Same change as the upstream PR (branch
feature/compact-night-hours on bartfaizoli76/webmail) - see that commit
for full technical details (new lib/calendar-time-scale.ts TimeScale
abstraction, generalized useTimeGridInteractions, new settings-store
field + toggle, en/hu locale strings + English placeholders for the
rest, new unit tests).
Verified post-merge: npm run typecheck (clean), npm run lint (0 errors,
same 7 pre-existing warnings), npx vitest run (3827 tests passed).
…night-hours mode User feedback: the night band alone was compact enough, but the day band (16h * full HOUR_HEIGHT) is still taller than most viewports on its own, so the whole day still didn't fit without scrolling. New COMPACT_DAY_HOUR_HEIGHT (32px week / 34px day, vs the normal 60/64) used for the day band specifically when the setting is on - new totals ~672px (week) / ~720px (day), down from ~1120/~1140.
…alendar occurrences When editing/deleting a recurring event whose master couldn't be found in the already-loaded events list, findMasterEvent falls back to a CalendarEvent/query by UID - but never told it which account to search, so it always queried the currently active account instead of the occurrence's actual owning account. For an event living on someone else's shared calendar (a completely normal case once cross-account calendar sharing is in play), the query always came back empty, making edits/deletes on such events silently fail even though the user has full write access to the calendar. Fix: pass occurrence.accountId as the query's targetAccountId (and resolve the right JMAP client via localAccountId first, mirroring the pattern already used a few lines above in the same function for the server-expanded-instance case).
…le id
The account-targeting fix wasn't enough on its own: a bare, unprefixed
JMAP id (e.g. 'k') looks like it belongs to the currently active
account to resolveMutationTarget (calendar-store.ts), which then
silently falls back to mutating against the wrong account instead of
erroring - so the subsequent update/delete call appeared to succeed
(no error toast) but actually touched nothing, exactly matching what
was observed against bulwark-tst.
Switched to queryAllCalendarEvents, which searches every account this
session has calendar access to AND already prefixes non-primary-account
results as `${accountId}:${id}` - the same convention the store's own
multi-account event list uses - so the returned master's id resolves
correctly wherever it's passed next.
…nt id
The previous fix (queryAllCalendarEvents + account-prefixed id) still
wasn't enough: under client-side recurrence expansion (any server
without synthetic-id support, e.g. pre-0.16.20 Stalwart), the bare
master is never itself present in the store - only its dated
occurrences (id `${master.id}:${recurrenceId}`) are. resolveMutationTarget
(calendar-store.ts) resolves purely by looking up the given id in the
store; a freshly-queried master's own id, however correctly account-
prefixed, will never match one of those occurrence ids, so it falls
through resolveMutationTarget's last-resort branch (bare id, no target
account) - hence the observed "notFound" JMAP error, confirmed via the
browser console this time (previously it silently no-op'd instead).
Fix: return the found master's real data but with id overridden to the
*occurrence's own* id (always store-resident, and resolveMutationTarget
already resolves any occurrence of a series to the true master via its
originalId/accountId fields - both already correctly set on it by
expandRecurringEvents when the range was first fetched). Mirrors the
analogous store-id substitution the isServerRecurrenceInstance branch a
few lines above already does, for the exact same reason.
… rehydration The shared-color auto-assign effect ran as soon as `calendars` loaded, without waiting for the settings store's localStorage rehydration to finish. If calendars loaded before rehydration completed, every shared calendar looked like it was still missing a color override (the store was still on its pre-hydration empty default), so the effect immediately picked and persisted fresh random colors - clobbering whatever the user had actually saved moments earlier when the real persisted value loaded in. Symptom: shared calendar colors changing on every app restart, and picked colors not sticking. Fix: track useSettingsStore's persist hydration status (persist.hasHydrated()/onFinishHydration()) and gate the auto-assign effect on it.
Week view 32px -> 36px, day view 34px -> 38px, per user feedback.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
New opt-in Calendar setting "Compact night hours" (default off): shrinks the 22:00-06:00 band in the day/week views, and (per feedback while testing) the day band too, so a full day fits with noticeably less scrolling.
New
lib/calendar-time-scale.ts: a smallTimeScaleabstraction (minutesToY/yToMinutes/totalHeight) with a linear implementation (existing behavior) and a new piecewise one for the compact mode.useTimeGridInteractionsnow takes aTimeScaleinstead of a flathourHeight, so drag-to-create/resize/DnD-drop all stay pixel-accurate under the non-linear scale (deltas convert through absolute Y positions, not a flat ratio).Real bug fix, found while testing the above against a real family shared-calendar setup:
findMasterEventincalendar-app.tsx(resolves a recurring event's master before edit/delete, when it isn't already in the locally-loadedeventslist) had two compounding issues for events on someone else's shared calendar:CalendarEvent/set, which is what triggers this whole fallback path in the first place), the bare master is never itself present in the store - only its dated occurrences (id${master.id}:${recurrenceId}) are.resolveMutationTargetresolves purely by looking up the given id in the store, so a non-store-resident id falls through its last-resort branch (bare id, no target account) - the subsequent update/delete then silently "succeeds" (no thrown error) while actually touching nothing.Fix: return the found master's data with
idoverridden to the occurrence's own id (always store-resident, andresolveMutationTargetalready resolves any occurrence of a series to the true master via itsoriginalId/accountId) - mirrors what the function'sisServerRecurrenceInstancebranch already does for the analogous reason.Minor: the shared-calendar auto-color-assignment effect could race the settings store's persist rehydration on load (assigns a fresh random color if it runs before the real saved one loads in) - now gated on
useSettingsStore.persist.hasHydrated().Test plan
shareWith), create a recurring event on the owner's account, then as the sharee try editing/deleting an occurrence (scope: this / this-and-future / all) - should actually apply, not silently no-opnpm run typecheck,npm run lint,npx vitest run- all clean in this branch (3838 tests passed when last run)🤖 Generated with Claude Code