feat(web): move notification indicator icons to top-right of bell icon #977
Conversation
previously, icons were placed next to bell icon because the status indicators were not accessible to color-blind users. this commit replaces circular status indicators with the icons.
the pulse was initially added to provide visual feedback when: 1. a new notification arrived 2. an alert notification was unread because we began using the legacy notify script, we now get a toast on new notifications. re:2, feedback on the pulse was mixed, so i'm removing it.
WalkthroughThe changes in Changes
Possibly related PRs
Suggested reviewers
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
web/components/Notifications/Indicator.vue (2)
44-47: Consider enhancing the unread indicator's accessibilityWhile the warning and alert states use distinct icons, the basic unread state still relies solely on a green dot. Consider using a small icon (e.g., a dot with rays) to make it more distinguishable for color-blind users.
- class="absolute top-0 right-0 size-2.5 rounded-full border border-neutral-800 bg-unraid-green" + class="absolute top-0 right-0 size-2.5 flex items-center justify-center" + > + <span class="block size-2 rounded-full border border-neutral-800 bg-unraid-green" /> + <span class="absolute size-full animate-ping-slow opacity-75 rounded-full bg-unraid-green"
42-52: Add ARIA attributes for screen readersTo improve accessibility for screen readers, consider adding appropriate ARIA attributes to convey the notification status.
- <div class="relative"> + <div class="relative" role="status" :aria-label="indicatorLevel ? `Notifications: ${indicatorLevel.toLowerCase()}` : 'No new notifications'"> <BellIcon class="w-6 h-6" /> <div v-if="indicatorLevel === 'UNREAD'" class="absolute top-0 right-0 size-2.5 rounded-full border border-neutral-800 bg-unraid-green" + aria-hidden="true" /> <component :is="icon.component" v-else-if="icon && indicatorLevel" :class="cn('absolute -top-1 -right-1 size-4 rounded-full', icon.color)" + aria-hidden="true" /> </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
web/components/Notifications/Indicator.vue(1 hunks)
🔇 Additional comments (1)
web/components/Notifications/Indicator.vue (1)
Line range hint 20-33: LGTM! Good use of semantic color classes for accessibility.
The implementation uses semantic color classes (text-yellow-500, text-unraid-red) which is good for maintainability and accessibility. The icon choices (ExclamationTriangleIcon, ShieldExclamationIcon) also provide clear visual distinction beyond just color, which aligns well with the PR's accessibility goals.
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
previously, icons were placed next to bell icon because the status indicators were not accessible to color-blind users. this commit replaces circular status indicators with the icons.
Summary by CodeRabbit
New Features
Bug Fixes
Style