feat(ui): phase 5 — modal/drawer exit, OTP auto-submit, icon swaps (#751) - #847
Open
alex-dembele wants to merge 7 commits into
Open
alex-dembele wants to merge 7 commits into
alex-dembele wants to merge 7 commits into
Conversation
Both the art and UX specs for phase 5 recommend dropping it. Cutting an item from the issue is the owner's call, so it is raised rather than decided. Signed-off-by: alex-dembele <alexandredembele16@gmail.com>
`if (!open) return null` cut every close instantly, so a create/edit modal that closes on mutation success never had an exit to play, and any enter/exit was one-directional keyframe animation rather than a reversible transition. Both now stay mounted at `data-state="closed"` through their exit and unmount via useExitTimer (byte-identical to the #825 branch's version, for a clean merge), with the CSS driven off `data-state` rather than a mount/unmount keyframe so a reopen mid-exit reverses the same transition instead of restarting one. Scrim and panel motion move from `motion-safe:animate-or-*` keyframes to plain CSS transitions in index.css (.or-scrim/.or-modal-panel/.or-drawer- panel), using the asymmetric-duration trick so entering and leaving can carry different tokens without a Tailwind duration-variant clash. useDismissableLayer keeps releasing focus/trap/scroll-lock on `open` itself, so that happens at the start of the close, not after the timer. Signed-off-by: alex-dembele <alexandredembele16@gmail.com>
OtpField's ring only transitioned border-color, so it jumped between boxes instead of sliding, and a filled segment's character popped in rather than fading (--motion-hover for the ring, --motion-press for the glyph, both plain transitions). Behaviourally, onComplete existed on OtpField and was unit-tested but neither MFA enrolment call site wired it up — AuthScreen's MFAEnrollment and MFAEnrollmentDialog both required a manual button press after the sixth digit. Both now submit on completion, guarded by `busy` so a second paste or a re-render in flight cannot double-fire, and the just-completed value is passed straight to submit rather than read off the component's `code` state, which is still one render behind at the moment onComplete fires. AuthScreen's enrolment field now also clears its error on edit, same as the dialog's already did. MFAEnrollmentDialog's OtpField gets an explicit `label` (the exact case its own doc comment names this dialog for) instead of relying on an implicit wrapping <label>. MFA login (AuthScreen's MFAChallenge) is deliberately untouched: it takes a 6-digit TOTP code OR a 12-character recovery code and is not an OtpField, so there is no onComplete to wire. Signed-off-by: alex-dembele <alexandredembele16@gmail.com>
…ght (#751) AppHeader.tsx's Sun/Moon toggle switched icons abruptly and its aria-label was the static "Toggle theme", which never told a screen-reader user what pressing it would do or that anything had changed. Both icons now sit in one grid cell and cross-fade (opacity + a slight scale, no rotation) on --motion-hover, and the accessible name names the result of pressing it — "Switch to light/dark theme" — so it flips with the theme the same way the icon does. New themeToLight/themeToDark keys in shared/uiStrings.ts, FR and EN, following the file's own established pattern for this chrome (it is kept separate from locales/*.json, consumed via useUIStrings()). The MFAEnrollmentDialog Copy→Check icon swap, the other half of this spec item, landed in the previous commit alongside the OTP wiring touching the same file — noted here rather than split after the fact. Signed-off-by: alex-dembele <alexandredembele16@gmail.com>
The policy/control modal (GovernancePage.tsx) used an `or-scalein` class with no matching CSS rule anywhere, so it popped in with no motion at all. MFAEnrollmentDialog.tsx never had an entrance class in the first place. Both are stopgapped with the house enter classes (motion-safe:animate-or-rise on the panel, motion-safe:animate-or-fadein on the scrim) rather than rebuilt: neither is the ds Modal, so neither gets the real data-state-driven exit transition Modal.tsx now has. Migrating them onto it is its own issue. Signed-off-by: alex-dembele <alexandredembele16@gmail.com>
frontend/src/components/layout/AppHeader.tsx, frontend/src/features/auth/__tests__/mfaEnrollmentDialog.test.tsx and frontend/src/shared/ds/__tests__/primitives.test.tsx only — no other file in the tree was reformatted. Signed-off-by: alex-dembele <alexandredembele16@gmail.com>
Only Modal had this test. The Drawer uses the same exit timer on its own duration, so it gets the same guard: same node, and the queued timer does not unmount a drawer that is open again. Signed-off-by: alex-dembele <alexandredembele16@gmail.com>
28 tasks
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.
Part of #751. This is phase 5, auth and micro-interactions. The branch is cut from
masterafter #819 and #823 and merges cleanly into the currentmaster(with #824). The spec was arbitrated from art-director and ux-designer and is posted on the issue.What changes
ModalandDrawer: a real exit.if (!open) return null, so every close was a hard cut.data-state="closed"through a CSS transition and unmount on a timer (useExitTimer, byte-identical to feat(notifications): phase 4 — bell count, dismissable panel, token toasts, banner stack (#751) #825's), at once under reduced motion.border-colorandbox-shadow, and characters fade in.onCompletenow submits in both MFA enrolment flows, guarded against double submit.onCompletefired before React appliedsetCode, so a stale closure was submitted.or-scaleinclass never had a CSS rule, so it had no motion at all; it now gets the houseor-rise/or-fadein.Verification
The runs were in an isolated
git worktree, because another session switched the shared checkout mid-run.Every new test fails without its change. The Drawer reopen-mid-exit test was proven against the pre-phase
Drawer.tsx.Live, real stack
A server was built from
masteron a throwaway Postgres 16 and Redis 7, with Vite serving this branch and Chromium samplinggetComputedStyleevery frame.data-state=closed, then unmounted; scroll lock released--motion-panel); focus inside the panelor-rise, 0→1 over ≈170 ms (it had no motion before)or-rise, 0→1 over ≈170 ms000000/auth/mfa/verifyrequest; the error shows andaria-invalid=true; one edit clears bothHonest remainders
VendorDetailPage:188,RemediationDetailPage:180,AuditDetailPage:194, andEntityDrawerHost:56. They need their own change.<body>: the trigger (the menu item) has gone by then. This is pre-existing, not introduced here.pointer-events:nonewhile closing. It is harmless, since it still covers the page for 120 ms.useEffect. It is proven to play live, but the mechanism is fragile if someone changes it touseLayoutEffect.