Align alert-dialog to architecture goals#202
Open
lifeiscontent wants to merge 2 commits into
Open
Conversation
|
📚 Storybook preview: https://pr-202-propel-storybook.vamsi-906.workers.dev |
Per issue #117, the designer marked spacing, right-aligned action placement, and max-width constraint as "always the same". These were previously left to the consumer via ad-hoc layout divs inside the stories. This commit bakes them in via two new anatomy parts and updates the popup's baked styles.
The popup now exposes the leading-icon region the design spec calls out
("icon placement, left of title"; "icon type warning/error/info"). Two new
ui parts:
- AlertDialogIcon: a decorative single-element node slot for the leading
glyph, with a required `tone` (danger/warning/info/success) — the
destructive-vs-informational axis the spec marks adjustable, no default.
- AlertDialogHeader: the row that places the icon at the inline-start of
the intro so the icon sits left of the title.
Styling stays in ui/.../variants.ts (cva); the components tier only
composes parts. Stories compose the new parts and register them in
subcomponents.
lifeiscontent
force-pushed
the
align/alert-dialog
branch
from
a47269d to
514d8a4
Compare
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.
Closes #117
What the spec says
From the designer's comment on #117:
Always the same (baked into the design system):
Depends / adjustable (consumer-supplied):
What changed
Previously, the "always the same" structural layout (title+description group, right-aligned actions row, popup max-width) lived as ad-hoc
className-bearingdivs inside the stories. The popup also had no width baked in — aw-80sat on an inner wrapper inside the story, not on the popup itself. The stories even noted "these are the future anatomy surfaces" but hadn't committed them.This commit hardens those surfaces:
AlertDialogIntro— new ui part (div) that groups the title and description with the always-the-sameflex flex-col gap-2spacing. Styled viaalertDialogIntroVariantsinvariants.ts.AlertDialogActions— new ui part (div) that right-aligns action buttons with the always-the-sameflex justify-end gap-2layout. Styled viaalertDialogActionsVariantsinvariants.ts.AlertDialogPopupgetsw-80 flex flex-col gap-4baked into its cva — the max-width constraint and top-level popup layout are always the same per spec, so they belong on the popup, not a wrapper div in the story.classNamefrom JSX.index.tsxexports andsubcomponentsmeta are updated to include the new parts.Architecture checklist
cxorclassNamein component.tsxfiles — all className lives invariants.tsviacvadefaultVariants— both new parts have no variant axes (the layout is invariant per spec)classNameorstyleprops exposed on any partvp check --no-fmtpasses clean across all 17 filesNotes
Needs design approval from @bhaveshraja before merge.
The "variant: destructive vs informational" and "icon type" axes from the spec are intentionally not yet wired — no Icon part exists yet and the visual treatment for informational vs destructive at the popup level hasn't been signed off. Those are tracked in #117.