feat: bell moves beside identity; deprecate the nav-row placement - #50
Merged
Conversation
planning#1133 §6 correction: the bell reads as a nav destination when it sits in the sidebar's nav list, but it's an ephemeral popover of recent items, not a triageable object with its own state — so it belongs beside identity instead, matching GitHub/Vercel/Figma/Slack. - New <Sidebar footerAccessory> prop and NotificationBell variant="footer": trailing icon beside identity when expanded, stacked above the avatar when the rail collapses, badge intact. - DOM order tracks visible order in both states (not a CSS-only reorder) so tab order and screen-reader reading order never diverge from what's on screen. - Dev-only warning when a bell ends up in footerAccessory without variant="footer", since the popover-clipping failure is otherwise silent. - variant="row" (v0.43) is deprecated, not removed — existing consumers are unaffected. Minor bump to 0.48.0.
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
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
Moves the notification bell out of the nav list and beside identity, per the corrected
wiki/concepts/design-system§6 (etamong-playground/planning#1185).The v0.43 placement put the bell in the nav list, where it read as a destination sitting next to Dashboard and Docs. It isn't one: our bell opens a popover of recent items and routes elsewhere — an ephemeral stream, not a triageable object with its own state. §6's test says that belongs next to identity, matching how GitHub, Vercel and Figma pair the bell with the avatar.
<Sidebar footerAccessory>+NotificationBell variant="footer". Expanded, the bell is a trailing icon beside theUserMenutrigger; collapsed to the 64px rail it stacks directly above the avatar-degraded identity control, badge intact. Below 720px nothing changes — apps mount it onNavigationBar's trailing edge.variant="row"is deprecated, not removed — ~8 apps use it, so the migration stays orderly.footerAccessoryis optional.Adversarial review caught a real accessibility bug
The first implementation rendered the bell trailing via CSS
flex-direction: row-reversewhile keeping it first in the DOM (so the collapsed column would stack without a second ruleset). That desyncs Tab and screen-reader linear order — both follow DOM — from visual order: keyboard users reached the bell before the identity control, and screen readers announced the unread count before the account control, inverting the exact hierarchy this placement exists to express. WCAG 2.4.3 / 1.3.2.Fixed by reordering the DOM in
Sidebarfrom afooterCollapsedflag, gated ontabletMode === "rail"rather than the rawcollapsedstate (which spuriously reads collapsed at tablet width indrawer/fullmodes). Plainflex; no CSS reorder trick. A Tab-order regression test now guards it — the suite previously asserted geometry and DOM order separately and never cross-checked them, which is why this passed review invisibly.Also added: a dev-only warning when a bell lands in
footerAccessorywithoutvariant="footer", since the failure mode otherwise is a silently clipped popover with no console signal.Deliberately not changed: the badge stays a pill at every width. §6's dot-degradation rule is scoped to
SidebarItem.badge(nav rows); the bell's own badge is bounded to"99+"by the component and always fits the 40px trigger, so the condition never fires. Two reviewers reached opposite conclusions on this, so the docs now say why.Verification
typecheck,build, vitest 136/136, showcase build, Playwright 18/18 — including the new Tab-order test and the rail-collapse visibility/badge/popover-open test. Screenshots reviewed at 1280×900 expanded, 1280×900 collapsed rail, and 390×844 in both themes.Relates to etamong-playground/planning#1133