fix: address UI/UX issues #823, #825, #827, #828 - #894
Conversation
…-IV#828 - NotificationSettings: add save confirmation toast (success/error) and inline error state on preference toggle - CampaignTabs: add unread indicator badge on tabs with new content since last visit, using localStorage to track last-viewed timestamps; clear indicators when tab is opened - AsyncButtonContent: add aria-live=polite region with aria-atomic for accessible loading announcement, and aria-hidden on spinner - InstallFreighterModal: detect mobile user agents and show mobile-appropriate guidance instead of extension install link Co-authored-by: adeniran19-maker <adeniran19-maker@users.noreply.github.com>
|
@adeniran19-maker 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! 🚀 |
|
Auto-review failed (API error). Leaving PR for human review. |
davidmaronio
left a comment
davidmaronio
left a comment
There was a problem hiding this comment.
good scope discipline for a multi-issue PR: each of the four fixes is confined to its own component plus a matching test file, and there is no overlap with #1002 (that PR covers #821, #822, #824, #826; this one covers #823, #825, #827, #828). checking each claimed fix:
#825 (AsyncButtonContent) is real and well done: aria-live polite region, aria-atomic, spinner hidden with aria-hidden, tests cover the toggle. #827 (InstallFreighterModal) is real: mobile ua detection, dedicated guidance copy, install/retry buttons hidden on mobile, tests for iphone and android uas. #823 and #828 need work:
- src/components/NotificationSettings.tsx:50-56 calls t("saveSuccess") and t("saveError") but the Notifications namespace in messages/en.json has no saveSuccess or saveError keys and this PR does not add them (messages/*.json is untouched). next-intl will render the raw key or throw. please add both keys to en.json and es.json.
- src/components/CampaignTabs.tsx:73-86 handleTabChange stamps setAllLastViewed for every tab on any tab click, so opening updates also clears the comments indicator. that defeats the per-tab indicator from #823; only the opened tab's timestamp should be updated.
- src/components/CampaignTabs.tsx:60-62 the unread heuristic is only campaign.created_at, so both tabs show the same badge and it never reflects actual new updates or comments. #823 asks to indicate which tab has new content; at minimum use the latest update timestamp and latest comment timestamp per tab, or state the limitation in the PR and issue.
- src/tests/components/CampaignTabs.test.tsx:110 the "clears the indicator" test clicks the comments tab and asserts the updates badge cleared, which only passes because of the bug in item 2. after fixing, assert the opened tab clears and the other tab keeps its badge.
typecheck/unit/build failures are partly the stale pre-#875 base, but the missing i18n keys are real, so please rebase and fix the above together.
|
Auto-review failed (API error). Leaving PR for human review. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| const showMobileGuidance = mobile && !browser.supported; | ||
|
|
||
| return ( | ||
| <div className="fixed inset-0 z-50 flex items-center justify-center bg-black/50 backdrop-blur-sm"> |
|
|
||
| return ( | ||
| <div className="fixed inset-0 z-50 flex items-center justify-center bg-black/50 backdrop-blur-sm"> | ||
| <div className="mx-4 w-full max-w-md rounded-2xl bg-white p-6 shadow-xl dark:bg-zinc-900"> |
| return ( | ||
| <div className="fixed inset-0 z-50 flex items-center justify-center bg-black/50 backdrop-blur-sm"> | ||
| <div className="mx-4 w-full max-w-md rounded-2xl bg-white p-6 shadow-xl dark:bg-zinc-900"> | ||
| <div className="text-center"> |
| </div> | ||
| )} | ||
|
|
||
| <div className="mt-6 space-y-3"> |
4 participants
Summary
This PR addresses four UI/UX issues reported in the Stellar Wave program:
Closes #823 — CampaignTabs does not indicate which tab has new content
Closes #825 — AsyncButtonContent spinner has no accessible loading announcement
Closes #827 — InstallFreighterModal deep link does not distinguish mobile vs desktop browsers
Closes #828 — NotificationSettings has no explicit save confirmation
#823 — CampaignTabs does not indicate which tab has new content
Added unread indicator badges on tabs that have new content since the visitor's last view. Uses localStorage to track last-viewed timestamps per campaign per tab. Clears all indicators when any tab is opened (visitor has seen the activity).
Files changed:
src/components/CampaignTabs.tsx— AddedlastViewedTimestampsstate,getLastViewed/setAllLastViewedhelpers, andhasUnreadcallback; maps tabs to includecount: 1when unreadsrc/__tests__/components/CampaignTabs.test.tsx— New tests for indicator appearing, clearing on tab open, and tab switching#825 — AsyncButtonContent spinner has no accessible loading announcement
Added
aria-live="polite"region witharia-atomic="true"that announces loading state changes to screen readers. Spinner is markedaria-hidden="true"to avoid duplicate announcements. Only announces on state transition via the aria-live region.Files changed:
src/components/AsyncButtonContent.tsx— AddeduseId-basedaria-live="polite"span; shows "Ready" when idle andpendingLabelwhen pending; spinner markedaria-hidden="true"src/__tests__/components/AsyncButtonContent.test.tsx— New tests for idle/pending rendering and aria-live region toggling#827 — InstallFreighterModal deep link does not distinguish mobile vs desktop browsers
Added mobile user agent detection via
isMobile()function. On mobile browsers, shows mobile-appropriate guidance ("Use a desktop browser") instead of the extension install link. Desktop install flow remains unchanged.Files changed:
src/components/InstallFreighterModal.tsx— AddedisMobile()helper; conditionally shows mobile guidance, hides install/retry buttons on mobile; changes icon and heading for mobilesrc/__tests__/components/InstallFreighterModal.test.tsx— New tests for desktop render, mobile guidance, and close behavior#828 — NotificationSettings has no explicit save confirmation
Integrated
useToastto show success toast on preference save and error toast on failure. Added inline error state with error message display if save fails.Files changed:
src/components/NotificationSettings.tsx— AddeduseToasthook (showSuccess,showError),saveErrorstate, inline error alert, andaria-expandedon settings buttonsrc/__tests__/components/NotificationSettings.test.tsx— New tests for rendering, toggling with localStorage persistence, and no-wallet stateValidation