Uh oh!
There was an error while loading. Please reload this page.
redesign(ui): plan-reminder form matches the reference sheet layout - #2004
Merged
Conversation
Round 2 on WAWQAQ's layout feedback — the first pass got the content
shape right but not the frame. Three deltas closed:
- The form is a full-height RIGHT SHEET now, not a centered modal:
Dialog pinned with position {top,right,bottom: 0}, rounded on the
exposed edge only, whisper backdrop so the task list stays readable
behind it. Focus trap / Escape / close-guard invariants ride along
unchanged since it is still the same Astryx Dialog.
- Each group's rows sit in an INSET ROUNDED CARD (hairline border +
radius + inline padding), the reference's grouped-row container,
instead of open edge-to-edge rows.
- Enum values render as QUIET "value ⌄" menu triggers (ghost
DropdownMenu whose label is the current option, row label carried in
the trigger's aria-label, check mark on the selected item) instead
of bordered Selector boxes. Recurrence / delivery method / platform
all switch; datetime, cron and chat-id stay real inputs.
- Footer reduces to the single primary Create, bottom-right — the
sheet's X and Escape are the cancel affordances, as in the
reference.
Verified: build, typecheck, check-dead-css, check-a11y/copy/console,
Storybook smoke (71 renders x 3 viewports), rendered screenshot of the
open sheet posted in the work thread.Uh oh!
There was an error while loading. Please reload this page.
jackwener added a commit
that referenced
this pull request
Aug 3, 2026
Adversarial review of #2002/#2004 (correctness + design lenses, every finding verified before fixing). Confirmed regressions, fixed: - Autofocus was broken: raw `autoFocus` fires at React commit, before showModal() makes the dialog visible, so focus silently landed on the first button. The title input now wears `data-autofocus`, the hook Astryx Dialog actually queries after opening (this is also what the plan-reminders e2e asserts). - The raw title input carried none of the a11y wiring TextInput used to provide — aria-required / aria-invalid / aria-describedby now point at Field's generated status id, and Field gets isRequired back, so the validation message is announced again. - The `::backdrop` override derived its color from --foreground, which inverts in dark mode (a whitening veil over a dark UI). It derives from --background now — light wash over light, dark scrim over dark — and the sheet gains a hairline inline-start border so dark mode separates by ring before shadow. - The form's own `overflow: auto` was a no-op inside LayoutContent's `overflow: clip`; tall variants (cron + bot delivery) clipped the Chat ID row and Create with no way to scroll. LayoutContent is the single scroll region now (isScrollable), between the pinned header and footer Layout slots. Design-review completions: - 时间 row: hand-typed `YYYY-MM-DDTHH:mm` TextInput → Astryx DateTimeInput (compact, 24h, 5-min stepping, full-row width); the sheet widens 440 → 480 so the date+time pair fits its min-content with proper insets. - The inset group cards gain a real fill (--muted) — a border on the same surface color read as a bare outlined rectangle, not an inset. - The borderless title gets a :focus-visible ring (it was the one control with no focus indicator); its placeholder moves to the platform text-secondary token (the hand mix was below 4.5:1). - Group labels step up to the label type role so the kicker/label ladder reads as two tiers; presets keep role="group"; the sheet slides in from the right (Astryx's center scale-pop detached a 100dvh element from three window edges mid-animation). - Dead copy keys pruned (eyebrow, cancel, field.delivery, templatesAriaLabel); two classNames that landed on the menu popover instead of the trigger removed. Verified: build, typecheck, check-dead-css, check-a11y/copy/console, plan-reminder unit tests (9/9), Storybook smoke (74 renders x 3 viewports), rendered screenshot of the open sheet re-checked.
jackwener added a commit
that referenced
this pull request
Aug 3, 2026
* fix(ui): deep-review round over the plan-reminder sheet Adversarial review of #2002/#2004 (correctness + design lenses, every finding verified before fixing). Confirmed regressions, fixed: - Autofocus was broken: raw `autoFocus` fires at React commit, before showModal() makes the dialog visible, so focus silently landed on the first button. The title input now wears `data-autofocus`, the hook Astryx Dialog actually queries after opening (this is also what the plan-reminders e2e asserts). - The raw title input carried none of the a11y wiring TextInput used to provide — aria-required / aria-invalid / aria-describedby now point at Field's generated status id, and Field gets isRequired back, so the validation message is announced again. - The `::backdrop` override derived its color from --foreground, which inverts in dark mode (a whitening veil over a dark UI). It derives from --background now — light wash over light, dark scrim over dark — and the sheet gains a hairline inline-start border so dark mode separates by ring before shadow. - The form's own `overflow: auto` was a no-op inside LayoutContent's `overflow: clip`; tall variants (cron + bot delivery) clipped the Chat ID row and Create with no way to scroll. LayoutContent is the single scroll region now (isScrollable), between the pinned header and footer Layout slots. Design-review completions: - 时间 row: hand-typed `YYYY-MM-DDTHH:mm` TextInput → Astryx DateTimeInput (compact, 24h, 5-min stepping, full-row width); the sheet widens 440 → 480 so the date+time pair fits its min-content with proper insets. - The inset group cards gain a real fill (--muted) — a border on the same surface color read as a bare outlined rectangle, not an inset. - The borderless title gets a :focus-visible ring (it was the one control with no focus indicator); its placeholder moves to the platform text-secondary token (the hand mix was below 4.5:1). - Group labels step up to the label type role so the kicker/label ladder reads as two tiers; presets keep role="group"; the sheet slides in from the right (Astryx's center scale-pop detached a 100dvh element from three window edges mid-animation). - Dead copy keys pruned (eyebrow, cancel, field.delivery, templatesAriaLabel); two classNames that landed on the menu popover instead of the trigger removed. Verified: build, typecheck, check-dead-css, check-a11y/copy/console, plan-reminder unit tests (9/9), Storybook smoke (74 renders x 3 viewports), rendered screenshot of the open sheet re-checked. * redesign(ui): plan-reminder form becomes a true split-view panel WAWQAQ round 3: the sheet was pinned to the WINDOW edge while the page content is width-clamped, leaving a dead band between the list and the form — the reference has none: its create panel sits flush against the list pane, one divider between, non-modal. - The form drops the <dialog> overlay entirely. It is an in-page <aside> now: when open, .maka-plan-panel becomes a two-column grid (list keeps flowing in column one; the panel takes minmax(380px, 460px) flush right) with a single hairline seam. Non-modal role="dialog" + aria-labelledby keeps the accessible name the e2e locates; Escape closes through the same guarded path via an onKeyDown at the panel root; the title autofocuses from a mount effect (the component still remounts per form session). - The aside is position: sticky with its own pinned header/footer and the form as the single scroll region between them, so the list can scroll independently — matching the reference's split behavior. - The sheet-era CSS (window-pinned dialog, custom ::backdrop) is retired; the slide-in animation survives on the aside. Verified: build, typecheck, check-dead-css, check-a11y/copy/console, plan-reminder unit tests 9/9, Storybook smoke (74 renders x 3 viewports), rendered screenshot re-checked against the reference. * fix(ui): plan form panel hands focus back to its opener on close The Astryx Dialog used to capture the focused element at open and restore it on close; the split-view aside dropped that contract, so Escape from the edit panel left focus on <body> — caught by the e2e that expects the row's menu trigger to regain focus. The open effect now captures the opener (the panel has already re-focused the triggering row control by then) and its cleanup hands focus back when the panel closes. Verified locally: both plan-reminders e2e tests pass.
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 freeto 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.
Round 2 on the 新建定时任务 redesign, per the feedback that #2002's layout didn't match the reference. Three deltas closed:
position={{top,right,bottom: 0}}), rounded on the exposed edge only, with a whisper backdrop so the task list stays readable behind it. Still the same Astryx Dialog, so focus trap / Escape / submit close-guard invariants are untouched.aria-label, check mark on the selected item), instead of bordered Selector boxes. Datetime / cron / chat-id stay real inputs.Footer reduces to the single primary Create bottom-right; the sheet's X and Escape are the cancel affordances, as in the reference.
Rendered screenshot of the open sheet posted in the work thread for review.
Verification: build ✅ typecheck ✅ check-dead-css ✅ check-a11y/copy/console ✅ Storybook smoke 71 renders × 3 viewports ✅