Fix "Mark all read" in inbox dropdown to not navigate#106
Conversation
The inbox dropdown's "Mark all read" button was a regular form POST with
`data: { turbo: false }`, which redirected to /notifications and forced
a full page navigation away from whatever page the user was on.
Now the form submits via Turbo and the controller responds with a
turbo_stream that:
- Updates #inbox-badge to "0"
- Replaces the #inbox-panel turbo frame with the refreshed panel
(showing the empty state)
The user stays on the current page, the dropdown stays open, and the
unread count clears immediately. The HTML format on /notifications
keeps its redirect behavior unchanged.
Generated with Amp
Amp-Thread-ID: https://ampcode.com/threads/T-019df4ad-2113-775d-bc20-300e6948aa32
Co-authored-by: Amp <amp@ampcode.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e29d7eb94
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @unread_count = 0 | ||
| render turbo_stream: [ | ||
| turbo_stream.update("inbox-badge", "0"), | ||
| turbo_stream.replace("inbox-panel", partial: "coplan/notifications/panel") |
There was a problem hiding this comment.
Use actual unread count in turbo-stream response
The turbo-stream branch hard-codes the unread badge and panel state to zero (@unread_count = 0 and turbo_stream.update("inbox-badge", "0")) instead of deriving them from the database after update_all. If a new notification is created for this user during the request window, the response will render unread items while simultaneously setting the badge to 0 and hiding the "Mark all read" button, leaving the dropdown in an inconsistent state until another refresh/broadcast occurs.
Useful? React with 👍 / 👎.
Summary
The inbox dropdown's "Mark all read" button was a regular form POST with
data: { turbo: false }, which redirected to/notificationsand forced a full page navigation away from whatever page the user was on. This made the action feel heavyweight — clicking it pulled you out of the current plan/page.This change makes it fire-and-forget: click → unread count goes to 0 → dropdown stays open showing the empty state → user remains on the page they were viewing.
Approach
Per AGENTS.md ("Prefer server-rendered HTML with standard Stimulus bindings"), this uses Turbo Streams rather than client-side JS clearing.
engine/app/views/coplan/notifications/_panel.html.erb): swapdata: { turbo: false }→form: { data: { turbo_stream: true } }sobutton_tosubmits with a turbo-stream Accept header.engine/app/controllers/coplan/notifications_controller.rb#mark_all_read): addrespond_towithformat.turbo_streamthat returns two streams:turbo_stream.updateof#inbox-badge→"0"turbo_stream.replaceof#inbox-panelwith the refreshed panel partial (which renders the empty state since there are no unread notifications)format.html { redirect_to notifications_path }is preserved for the index page's "Mark all read" button.Verification
bundle exec rspec— all 791 examples pass, including a new spec for the turbo_stream response.text/vnd.turbo-stream.html, contains anupdateaction targetinginbox-badge, and areplaceaction targetinginbox-panel.Generated with Amp