feat(web): clear notifications indicator after opening sidebar #1001
Conversation
WalkthroughThe pull request introduces a new Changes
Sequence DiagramsequenceDiagram
participant User
participant Sidebar
participant Indicator
User->>Sidebar: Click Notifications
Sidebar->>Sidebar: prepareToViewNotifications()
Sidebar->>Indicator: Pass seen=true
Indicator->>Indicator: Update visibility
Possibly related PRs
Poem
Warning Review ran into problems🔥 ProblemsGitHub Actions: Resource not accessible by integration - https://docs.github.com/rest/actions/workflow-runs#list-workflow-runs-for-a-repository. Please grant the required permissions to the CodeRabbit GitHub App under the organization or repository settings. 🪧 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
🧹 Nitpick comments (1)
web/components/Notifications/Indicator.vue (1)
6-6: Ensure a default for the new boolean prop.
If theseenprop remains undefined,!seenwill evaluate totrue, which might incorrectly display the indicator. Consider a default value offalseor making the prop required.-const props = defineProps<{ overview?: OverviewQuery['notifications']['overview'], seen?: boolean }>(); +const props = defineProps<{ + overview?: OverviewQuery['notifications']['overview']; + seen?: boolean; +}>({ + seen: { + type: Boolean, + default: false + } +});
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (2)
web/components/Notifications/Indicator.vue(2 hunks)web/components/Notifications/Sidebar.vue(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build and Test API
- GitHub Check: Build Web App
🔇 Additional comments (6)
web/components/Notifications/Indicator.vue (2)
45-45: Conditional logic for the unread indicator looks correct.
No issues identified; the condition effectively avoids displaying the badge if notifications have been seen or if there are no unread notifications.
50-50: Good fallback for the icon.
Thev-else-iflogic ensures that when the indicator is not an unread badge but is a warning/alert, it will still display, providedseenisfalse. This approach is straightforward and clear.web/components/Notifications/Sidebar.vue (4)
42-45: Excellent approach to track notification visibility.
Defining a reactive refhasSeenNotificationselegantly captures whether the user has viewed their notifications.
46-55: Watch logic effectively resets the seen state.
When new unread notifications arrive,hasSeenNotificationsis promptly reset tofalse. This is a solid approach, but please confirm you’ve considered scenarios where unread might decrease and then increase again.
57-60: Well-named function to prepare the UI for viewing.
prepareToViewNotifications()is clear in intent. The sequence of callingdetermineTeleportTarget()before marking notifications as seen seems logical.
65-67: Prop binding aligns with new indicator logic.
Passing:seen="hasSeenNotifications"to theNotificationsIndicatorcompletes the feature, ensuring the indicator immediately reflects whether notifications have been viewed.
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
Clears notification indicator in upc after opening sidebar (including warning/alert state). Comes back if new notifications arrive.
Summary by CodeRabbit
New Features
seenstateBug Fixes
Refactor