Skip to content

fix(header): mark the Alerts menu item with a check while alerts are on - #1000

Merged
devswha merged 1 commit into
mainfrom
fix/alerts-menu-check
Oct 14, 2026
Merged

devswha merged 1 commit into
mainfrom
fix/alerts-menu-check

Conversation

@devswha

@devswha devswha commented Oct 13, 2026

Copy link
Copy Markdown
Owner

Closes #922

Change

The header menu's Alerts row is a switch (menuitemcheckbox, a pressed button in the phone's sheet), but it drew only an icon, a label and a state in words, so it read as a link to Settings → Alerts while a click turned alerts on or off at once.

A menu item with checked: true now draws a check mark after its hint, in the popover and in the sheet. Alerts is the only such item today. No CSS was added: the mark takes the menu's existing icon size and color.

Not in this PR, from the issue's proposal: a stateful label ("Alerts: On/Off") and an undo toast after turning alerts off. The hint already says the state in words.

Validation

  • bun run typecheck, and the i18n, conventions and changelog-fold unit tests: pass (macOS arm64, Bun 1.4.0).
  • Browser: bun run check run --build bun scripts/ui-regression.ts: 65 PASS.
    It runs the Alerts item through scripts/header-more.ts, which checks the item's state against its words.
  • Seen on the demo build, desktop (1280x800) and phone (390x844): the mark shows beside "On in this tab" and is gone beside "Off on this device" after a click.
  • Not run locally: the full integration and browser lanes.

@coderabbitai

coderabbitai Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: devswha/herdr-web-ui/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: cd33c516-c103-4310-b91b-31b82998d0fb
📥 Commits

Reviewing files that changed from the base of the PR and between 8685ff1 and f433028.

📒 Files selected for processing: 2
  • changelog.d/alerts-menu-check.md
  • src/components/RowMenu.tsx

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 0 remain after this review.

📝 Summary

Summary by CodeRabbit

  • UI Improvements
    • The Alerts menu row now displays a check mark when alerts are enabled, clarifying its on/off state.

RowMenu now displays a check icon beside checked items in the phone sheet and desktop popover. The changelog describes the Alerts row's checked state.

Changes: Checked Menu State

Layer / File(s) Summary
Render checked items
src/components/RowMenu.tsx, changelog.d/alerts-menu-check.md
RowMenu imports the Lucide Check icon and renders it when an item's checked value is truthy in both menu layouts. The changelog describes the Alerts row's checked state.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix · Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to f4330

The check reflects the Alerts enabled state in both layouts; no actionable merge risk remains.

Pre-merge checks: Passed 3 · Failed 2

Failed Failed checks: 2 warnings

Check name Status Explanation Resolution
Linked Issues check Warning Issue #922 requests a visible toggle state and a stateful Alerts label. src/components/RowMenu.tsx now renders Check for checked items in both the popover and phone sheet, so the visual state is i… Update the Alerts item label to include its current state, with the required i18n coverage and automated tests. Keep the check mark in both menu surfaces.
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (1 skipped: 1 … Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 3 passed
Check name Status Explanation
Title check Passed The title clearly and concisely identifies the primary change: showing a check mark for the Alerts menu item when alerts are enabled.
Description check Passed The description includes the Change and Validation sections, explains the resulting behavior, records test results and manual verification, and notes which validation lanes were not run.
Out of Scope Changes check Passed The changes are limited to the shared RowMenu rendering and a changelog entry. The Check icon for checked items directly supports issue #922 in both the desktop popover and phone sheet. No unrelat…
Full details: Linked Issues check

Explanation

Issue #922 requests a visible toggle state and a stateful Alerts label. src/components/RowMenu.tsx now renders Check for checked items in both the popover and phone sheet, so the visual state is implemented. The item still receives a separate label and hint; this PR does not implement the requested stateful label such as Alerts: On/Off. The optional undo toast is not a required coding objective.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 2
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

gimso2x pushed a commit to gimso2x/herdr-web-ui that referenced this pull request Oct 14, 2026
## Change

CHANGELOG.md's `## [Unreleased]` lists are the repo's most frequent
merge conflict: every PR appends to the same Added/Changed/Fixed lists,
and a conflict resolver once pushed the markers (devswha#838). A PR now adds
one small file to `changelog.d/` instead, so two PRs never touch the
same lines.

- **Format** (`changelog.d/README.md`): a lowercase `name.md` holding
exactly the lines it would have in CHANGELOG.md: `###
Added|Changed|Removed|Fixed|Maintenance` headings, `- ` entries,
two-space continuations. No PR link or credit needed.
- **Fold** (`scripts/changelog-fold.ts`): the release PR runs `bun
scripts/changelog-fold.ts X.Y.Z [YYYY-MM-DD]` (the date defaults to the
local one). It turns the Unreleased notes into `## [X.Y.Z] - date` under
an empty `## [Unreleased]` (the shape of devswha#853), with the existing
Unreleased entries first, then the fragments sorted by file name,
sections in release order (any other heading the old notes used is kept
after them). It links each fragment's entries to the PR whose squash
merge added the file (from `git log --follow --diff-filter=A`, so a
later rename does not take the link) unless the entry already links that
PR, moves the compare links on, deletes the fragments, and prints what
still needs a link plus the reminder to add `by @login` credits. It
refuses rather than drops a line it cannot place.
- **Release gate**: `scripts/release-notes.ts`, which release.yml runs
before tagging, refuses while `changelog.d/` still holds a fragment (one
merged after the release PR folded them would otherwise ship without a
line in the notes), and says to move its entries into the version by
hand.
- **Transition**: the ~30 open PRs that still append to Unreleased need
no rework; their lines are folded together with the fragments.
CHANGELOG.md content is unchanged in this PR.
`scripts/release-notes.ts`, `server/release-notes.ts` and
`scripts/update-browser-qa.ts` read the same shape as before.
- **Guards** (`scripts/changelog-fold.test.ts`, unit suite): every
fragment parses (known section, at least one non-empty entry, no stray
text, lowercase `.md` name); CHANGELOG.md and the fragments hold no
conflict markers; today's Unreleased notes plus fragments fold for the
next release, so an old PR's malformed entry fails at its own PR rather
than at release. Failure messages name the file and line and point at
`changelog.d/README.md`. Not enforced yet: that a PR leaves CHANGELOG.md
alone. A ratchet or a `changes`-job check would fail the ~30 open PRs
that still append there (or turn main red when one of them merges on a
stale green run), so it is a planned follow-up once they have drained;
AGENTS.md says so.
- **Docs**: CONTRIBUTING.md (changelog bullet), AGENTS.md maintainer
workflow (one line, pointing at the test), docs/development.md Releasing
step 1, the PR template comment.

No CHANGELOG entry or fragment: not user-visible.

## Validation

- `bun run check fast`: workflow syntax, generated types, typecheck (the
fold script added to `tsconfig.tools.json`), build, unit (3032 pass, 0
fail, 8 skip).
- `bun test scripts/changelog-fold.test.ts
scripts/release-notes.test.ts`: 18 pass, including one where
`releaseNotes()` reads a folded changelog.
- `git log --follow --diff-filter=A` checked on a throwaway branch:
after `x (devswha#1000)` adds a fragment and `chore: rename (devswha#1001)` renames
it, it reports devswha#1000 (without `--follow`, devswha#1001).
- Dry run on this branch with a probe fragment: `bun
scripts/changelog-fold.ts 0.4.9 2026-10-13` changed CHANGELOG.md by only
the new heading, the probe line at the end of Fixed and the two compare
links (+5/-1), and deleted the fragment. Reverted afterwards.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
@devswha
devswha merged commit b95b185 into main Oct 14, 2026
21 of 25 checks passed
@devswha
devswha deleted the fix/alerts-menu-check branch October 14, 2026 00:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Header menu 'Alerts' looks like a Settings link but toggles alerts off immediately (no visual toggle state)

1 participant