[323] Design empty list display views for notification_bell - #361
Open
kingeligma wants to merge 5 commits into
Open
[323] Design empty list display views for notification_bell#361kingeligma wants to merge 5 commits into
kingeligma wants to merge 5 commits into
Conversation
Implements the navbar alert bell badge as a keyboard-operable (role=button) disclosure with ARIA compliance (Goldii-locks#320): accessible name, aria-haspopup/aria-expanded/aria-controls, aria-live announcement regions, aria-hidden on decorative glyphs, focus-visible rings and design-token contrast. Adds field error indicators and alerts (Goldii-locks#324): validation field configs render role=alert error text that toggles as validation triggers, wired via aria-describedby and counted toward the unread badge. Adds React Testing Library tests covering both requirements. Closes Goldii-locks#320 Closes Goldii-locks#324
Style hover, active/pressed, focus-visible, and disabled states on the bell trigger with Tailwind utilities: - hover background shift, active press feedback (bg + scale) with a faster press duration - focus-visible keyboard ring preserved with offset - disabled prop: native + aria-disabled, muted opacity, cursor-locked, inert hover/press/focus variants while the unread badge stays visible Adds notification_bell_interactive_states tests covering hover, press, focus-visible, keyboard toggling, and disabled trigger behavior. Also carries the suite-repair changes (SignatureTimeoutAlert useMemo, localStorage shim, freighter XDR fixtures, useWallet stubs) so CI runs green. Closes 321
Add descriptive placeholder UI for empty notification states: - Full-empty panel (no notifications, no validation fields) renders a premium placeholder with a decorative bell badge, a "You're all caught up." title, helper copy, announced via role=status. - Panel with validation fields but no notifications renders a compact dashed "No new notifications." placeholder beneath the field rows. - Placeholders are suppressed whenever notifications exist. Adds notification_bell_empty_state tests covering each layout and the suppression rules. Closes 323
There was a problem hiding this comment.
Pull request overview
Adds designed empty-state placeholder views for the notification_bell panel and expands the surrounding test/fixture infrastructure so the new UI states (and related wallet/multisig flows) remain testable in Vitest/jsdom.
Changes:
- Introduces
NotificationBell(with full-empty + notifications-empty placeholders) plus Storybook scenarios. - Adds dedicated test suites covering a11y, interactive states, and empty-list display suppression rules for
notification_bell. - Stabilizes test environment and wallet/multisig-related suites (jsdom
localStoragenormalization, wallet context stubs, and real Stellar XDR fixtures).
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
vitest.setup.ts |
Adds an in-memory localStorage fallback to avoid Node/jsdom storage-global inconsistencies impacting tests. |
app/components/SignatureTimeoutAlert.tsx |
Converts parse failure message to derived memoized state to avoid effect-driven state churn. |
app/components/notification_bell.tsx |
New NotificationBell component with empty-state layouts, badge counts, and ARIA roles/labels. |
app/components/notification_bell.stories.tsx |
Storybook stories for notification, validation, and empty-state variants. |
__tests__/signature_timeout_alert.test.tsx |
Stubs wallet context and wraps wallet-operation mutations in act() for RTL correctness. |
__tests__/notification_bell.test.tsx |
New a11y + validation + notification rendering tests for notification_bell. |
__tests__/notification_bell_interactive_states.test.tsx |
New tests for hover/active/focus-visible styling and disabled behavior. |
__tests__/notification_bell_empty_state.test.tsx |
New tests specifically for the empty-list placeholder layouts and suppression rules. |
__tests__/freighter_multisig_hook.test.ts |
Updates fixtures to generate real signed Stellar envelopes for parser-valid XDR. |
__tests__/freighter_connector.component.test.tsx |
Stubs wallet context to keep freighter connector logic testable without UMD/ESM import issues. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| * - `aria-haspopup` + `aria-expanded` expose the disclosure panel state. | ||
| * - `aria-label` accessible name; the bell glyph is `aria-hidden`. | ||
| * - `aria-live="polite"` region announces updates; errors use `role="alert"`. | ||
| * - `aria-invalid` + `aria-describedby` wire field errors to their messages. |
Comment on lines
+78
to
+87
| it("operates from the keyboard (Enter/Space activate the native button)", () => { | ||
| renderBell(); | ||
| const trigger = screen.getByRole("button"); | ||
| fireEvent.keyDown(trigger, { key: "Enter" }); | ||
| fireEvent.click(trigger); | ||
| expect(trigger).toHaveAttribute("aria-expanded", "true"); | ||
| fireEvent.keyDown(trigger, { key: " " }); | ||
| fireEvent.click(trigger); | ||
| expect(trigger).toHaveAttribute("aria-expanded", "false"); | ||
| }); |
Comment on lines
+41
to
+46
| it("is still keyboard-operable and toggles the panel on activation", () => { | ||
| expect(trigger).toHaveAttribute("aria-expanded", "false"); | ||
| fireEvent.keyDown(trigger, { key: "Enter" }); | ||
| fireEvent.click(trigger); | ||
| expect(trigger).toHaveAttribute("aria-expanded", "true"); | ||
| }); |
|
@kingeligma Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
…cing timers Three timeout tests attached promise.catch() only after awaiting timer advance, so the rejected outer promise could be snapshotted as unhandled by Node during the interleaved microtask gap, making vitest report 3 unhandled errors and exit non-zero despite all tests passing. Attach the handler immediately after runSignatureWithTimeout (mirroring the .rejects-based tests) to keep npm run test exiting cleanly.
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.
Closes #323
What
Designs descriptive placeholder UI for
notification_bellunder empty list states:role="status"(data-testid="notification-bell-empty-state").data-testid="notification-bell-notifications-empty").Tests
__tests__/notification_bell_empty_state.test.tsxcovers each layout plus the suppression rules (notifications present, alerts-only panels, mixed content).Notes
This branch stacks on #321's branch (
fix/321-...) which contains the notification_bell interactive states, and carries the shared suite-repair changes so CI stays green.Checks
lint, type-check,
npm run test(89 files / 1357 tests),next build— all pass.