Skip to content

Conversation

@elibosley
Copy link
Member

@elibosley elibosley commented Sep 23, 2025

Summary

  • replace the notification type dropdown with inline pill buttons for quick filtering
  • expose accessible role and pressed state on the new filter buttons

Testing

  • pnpm --filter @unraid/web lint

https://chatgpt.com/codex/tasks/task_e_68d184ad60348323b60c9b8e19146025

Summary by CodeRabbit

  • New Features

    • Notifications sidebar now uses a pill-style button group instead of a dropdown for filtering by importance/type.
    • One-tap filter buttons apply selections instantly and show active state for clearer, more accessible filtering.
    • Settings tooltip/actions moved alongside the new filter controls for a unified layout.
  • Chores

    • Removed debug output; no changes to public APIs or workflows.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 23, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Replaced the Select-based notification filter in Sidebar.vue with a pill-style button group driven by a filterOptions array; clicks assign importance directly and emit updates. Removed an incoming-notifications console.log and relocated the Settings tooltip/actions into the same filter container. No public API changes. (48 words)

Changes

Cohort / File(s) Summary
Notifications filtering UI & minor cleanup
web/src/components/Notifications/Sidebar.vue
Removed Select-based filter and its update-model-value mapping; added filterOptions array and pill-style button group that sets importance on click; removed a debugging console.log; moved Settings tooltip/actions into the same container; preserved external emits/API.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant Sidebar as Notifications Sidebar
  participant State as Local State (`importance`)
  participant Parent as Parent Component

  User->>Sidebar: Click filter button (pill)
  Sidebar->>State: set importance = selectedOption (or undefined for "All Types")
  Note right of Sidebar #DFF2E1: UI control changed from Select to pill button group\nSettings tooltip/actions now live beside filters
  Sidebar-->>Parent: Emit update:importance (API unchanged)
  Parent->>Parent: Apply filter to notifications
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I thump my paws—click, click, hooray!
Buttons bloom where dropdowns lay.
Filters hop with nimble grace,
Importance set, a tidy place.
In the burrow, signals flow—Carrot-approved UI glow! 🥕✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title clearly and concisely reflects the main change of refining notification filter controls by replacing the dropdown with inline pill buttons for filtering.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e3aa43b and f9111ac.

📒 Files selected for processing (1)
  • web/src/components/Notifications/Sidebar.vue (2 hunks)

Comment @coderabbitai help to get the list of available commands and usage tips.

@claude
Copy link

claude bot commented Sep 23, 2025

Claude encountered an error —— View job


I'll analyze this and get back to you.

@codecov
Copy link

codecov bot commented Sep 23, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 53.29%. Comparing base (b3ca40c) to head (f9111ac).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1718      +/-   ##
==========================================
+ Coverage   52.58%   53.29%   +0.71%     
==========================================
  Files         858      859       +1     
  Lines       48092    48246     +154     
  Branches     4792     4912     +120     
==========================================
+ Hits        25288    25714     +426     
+ Misses      22735    22463     -272     
  Partials       69       69              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
web/src/components/Notifications/Sidebar.vue (2)

48-53: Make the “All Types” option explicit to avoid relying on missing property semantics.

Define value: undefined for clarity and to prevent accidental regressions if this object is refactored.

-const filterOptions: Array<{ label: string; value?: Importance }> = [
-  { label: 'All Types' },
+const filterOptions: Array<{ label: string; value?: Importance }> = [
+  { label: 'All Types', value: undefined },
   { label: 'Alert', value: Importance.ALERT },
   { label: 'Info', value: Importance.INFO },
   { label: 'Warning', value: Importance.WARNING },
 ];

Optional: consider making this immutable for safer typing:
as const satisfies ReadonlyArray<{ label: string; value: Importance | undefined }> (if TS version supports satisfies).


205-220: ARIA: Label should reference “importance,” not “type”.

These pills filter by importance (Alert/Info/Warning). Update the accessible name for screen readers.

-  aria-label="Filter notifications by type"
+  aria-label="Filter notifications by importance"

Optional: since this is single‑select, a radiogroup/radio pattern can further improve keyboard navigation (arrow-key cycling) over discrete toggle buttons.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ed9a5c5 and 500b958.

📒 Files selected for processing (1)
  • web/src/components/Notifications/Sidebar.vue (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/components/**/*.vue

📄 CodeRabbit inference engine (.cursor/rules/web-testing-rules.mdc)

Some Vue files may require explicit imports like ref or computed due to Nuxt auto-imports not applying in tests

Files:

  • web/src/components/Notifications/Sidebar.vue

@elibosley elibosley force-pushed the codex/replace-dropdown-with-clickable-tags-in-notifications branch from 26f8730 to e53557e Compare September 23, 2025 15:41
@elibosley elibosley force-pushed the codex/replace-dropdown-with-clickable-tags-in-notifications branch from e53557e to e3aa43b Compare September 25, 2025 15:04
@github-actions
Copy link
Contributor

This plugin has been deployed to Cloudflare R2 and is available for testing.
Download it at this URL:

https://preview.dl.unraid.net/unraid-api/tag/PR1718/dynamix.unraid.net.plg

@elibosley elibosley merged commit 661865f into main Sep 26, 2025
12 of 14 checks passed
@elibosley elibosley deleted the codex/replace-dropdown-with-clickable-tags-in-notifications branch September 26, 2025 14:15
elibosley pushed a commit that referenced this pull request Sep 26, 2025
🤖 I have created a release *beep* *boop*
---


## [4.25.0](v4.24.1...v4.25.0)
(2025-09-26)


### Features

* add Tailwind scoping plugin and integrate into Vite config
([#1722](#1722))
([b7afaf4](b7afaf4))
* notification filter controls pill buttons
([#1718](#1718))
([661865f](661865f))


### Bug Fixes

* enable auth guard for nested fields - thanks
[@ingel81](https://github.com/ingel81)
([7bdeca8](7bdeca8))
* enhance user context validation in auth module
([#1726](#1726))
([cd5eff1](cd5eff1))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants