Skip to content

Fix "Mark all read" in inbox dropdown to not navigate#106

Merged
HamptonMakes merged 1 commit intomainfrom
hampton/notification-mark-all-as-read
May 5, 2026
Merged

Fix "Mark all read" in inbox dropdown to not navigate#106
HamptonMakes merged 1 commit intomainfrom
hampton/notification-mark-all-as-read

Conversation

@HamptonMakes
Copy link
Copy Markdown
Collaborator

Summary

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. 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.

  • View (engine/app/views/coplan/notifications/_panel.html.erb): swap data: { turbo: false }form: { data: { turbo_stream: true } } so button_to submits with a turbo-stream Accept header.
  • Controller (engine/app/controllers/coplan/notifications_controller.rb#mark_all_read): add respond_to with format.turbo_stream that returns two streams:
    • turbo_stream.update of #inbox-badge"0"
    • turbo_stream.replace of #inbox-panel with the refreshed panel partial (which renders the empty state since there are no unread notifications)
  • The existing 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.
  • The new spec asserts the response is text/vnd.turbo-stream.html, contains an update action targeting inbox-badge, and a replace action targeting inbox-panel.

Generated with Amp

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>
@HamptonMakes HamptonMakes marked this pull request as ready for review May 5, 2026 14:48
@HamptonMakes HamptonMakes merged commit fcc4aaf into main May 5, 2026
5 checks passed
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 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".

Comment on lines +54 to +57
@unread_count = 0
render turbo_stream: [
turbo_stream.update("inbox-badge", "0"),
turbo_stream.replace("inbox-panel", partial: "coplan/notifications/panel")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge 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 👍 / 👎.

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.

1 participant