[321] Add premium interactive states to notification_bell - #360
Open
kingeligma wants to merge 4 commits into
Open
[321] Add premium interactive states to notification_bell#360kingeligma wants to merge 4 commits into
kingeligma wants to merge 4 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
There was a problem hiding this comment.
Pull request overview
This PR introduces a new notification_bell client component with “premium” interactive states (hover/pressed/focus-visible/disabled) implemented via Tailwind utilities, and carries several test-suite stabilizations to keep CI green (notably around wallet context imports and XDR fixtures).
Changes:
- Add
NotificationBellcomponent with interactive/disabled styling, badge counts, and an associated Storybook story. - Add new test coverage for
notification_bell(a11y/validation/interactive states) and adjust existing tests to avoid brittle module-scope wallet-provider imports. - Improve test reliability by adding a Vitest
localStorageshim, switching SignatureTimeoutAlert parse error state to derived memoized state, and using real Stellar SDK-signed envelopes in relevant multisig hook tests.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
vitest.setup.ts |
Adds a localStorage fallback for jsdom tests across Node versions. |
app/components/SignatureTimeoutAlert.tsx |
Replaces effect-driven parse error state with memoized derived state. |
app/components/notification_bell.tsx |
Adds the new notification bell trigger/panel with interactive states, badge, and validation/notification rendering. |
app/components/notification_bell.stories.tsx |
Adds Storybook stories covering common bell states and content combinations. |
__tests__/signature_timeout_alert.test.tsx |
Stubs useWallet to avoid non-importable wallet kit deps and wraps store updates in act(). |
__tests__/notification_bell.test.tsx |
Adds tests for a11y behavior, badge counts, validation alerts, and notification rendering. |
__tests__/notification_bell_interactive_states.test.tsx |
Adds tests asserting presence of hover/active/focus/disabled Tailwind state classes and disabled inertness. |
__tests__/freighter_multisig_hook.test.ts |
Switches fixtures to real signed envelopes to exercise the actual SDK parser. |
__tests__/freighter_connector.component.test.tsx |
Stubs useWallet to keep freighter connector logic under test without importing the wallet kit at module scope. |
Suppressed comments (1)
app/components/notification_bell.tsx:117
- The panel visibility is controlled only by
open(hidden={!open}), so the panel can remain visible/interactable ifdisabledbecomes true after it was opened. If disabled should make the panel inert, its visibility should also account fordisabled.
hidden={!open}
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| <div className="relative inline-block"> | ||
| <button | ||
| type="button" | ||
| aria-label={label} |
| type="button" | ||
| aria-label={label} | ||
| aria-haspopup="dialog" | ||
| aria-expanded={open} |
|
@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 #321
What
Adds premium interactive states to the
notification_belltrigger using Tailwind utilities:disabledprop): nativedisabled+aria-disabled, muted opacity,cursor-not-allowed, and explicitly inert hover/press/focus variants; the unread badge and accessible name remain visibleTests
__tests__/notification_bell_interactive_states.test.tsxcovers the hover/press/focus classes, keyboard operability, and the full disabled matrix (attributes, styling, click/keyboard inertness, retained badge).Suite repair (carried to every derived branch)
The base suite is broken on
main; this branch carries the verified repairs (SignatureTimeoutAlert useMemo, localStorage shim, freighter XDR fixtures, useWallet stubs) so checks are green.Checks
lint, type-check,
npm run test(88 files / 1350 tests),next build— all pass.