[ako3131]

ref: https://app.clickup.com/t/9014802374/86bcghcxq

What changed

• Errors stay open until closed.
• Success messages hide after 6s (was 2s).
• Error messages queue up instead of overwriting each other.
  • Closing an error message reveals the next message.
• Repeats within 1s are ignored. A repeated success is ignored while the same one is still showing or waiting
• Blank messages are ignored.
• Errors close when you change pages.
• Successes and errors are in their own queueus. Successes show on the top of the screen, errors at the bottom. (see referenced click up link for a screen cap of this)

Follow-ups

• Corresponding change for navigation auto hide functionality in summit admin: feat: pass histroy to snackbar so it can close on a navigation fntechgit/summit-admin#1149
• A newer message replacing the current one feature were left out to keep this PR simple and can be added later.

[coderabbitai]

Important

• Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 3182f0d3-ab3c-4b57-b79c-33c3f6e9c6c8

Pre-merge checks: 1 · 1

Failed checks: 1 inconclusive

Check name	Status	Explanation	Resolution
Title check		This assessment has not completed. The check will be retried on the next pre-merge check run.

Passed checks: 1 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.

• 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

• X
• Mastodon
• Reddit
• LinkedIn

[Copilot]

🟡 Changes recommended

A notification arriving during an exit transition can prematurely replace the closing message, and the tests do not verify that replacement success notifications receive a full fresh auto-hide timeout.

2 open issues

• Add timer regression coverage for replaced success notifications · New
• Replacement interrupts the outgoing snackbar transition · New

What changed in this PR

This PR updates the shared snackbar component to keep errors visible until dismissed and give replacement messages their own timing.

Changes:

• Keeps non-success messages open while preserving the success timeout.
• Adds latest-message replacement handling and a one-second deduplication window.
• Expands tests for persistence, dismissal, replacement, and repeated messages.

File	Description
src/​utils/​constants.js	Adds the notification deduplication window.
src/​components/​mui/​SnackbarNotification/​index.js	Updates dismissal, replacement, deduplication, and timeout behavior.
src/​components/​mui/​__tests__/​snackbar-notification.test.js	Adds fake-timer tests for the updated snackbar behavior.

_{Review effort: Balanced}

[santipalenque]

hey @ako3131 code looks good and is an improvement from what we have, but I think there are still things to polish/improve that we can do in this same effort. Let me know if you want to discuss

[Copilot]

🟡 Changes recommended

The snackbar queue still drops batched notifications, prematurely dismisses queued successes, and allows waiting successes to delay newly arriving errors.

3 open issues

• New errors are queued behind waiting successes · New
• Queued successes close immediately when promoted · New
• Batched Redux notifications overwrite earlier messages · New

2 resolved since last review

• Replacement interrupts the outgoing snackbar transition
• Add timer regression coverage for replaced success notifications

_{Review effort: Balanced}

[smarcet]

@ako3131 Not a defect of this PR. The single-slot overwrite lives in the reducer (src/utils/reducers.js:52-53) and predates this change; the previous setMsgData(snackbarMessage) had the same limit. It only drops a message when two SET_SNACKBAR_MESSAGE dispatches land in the same React batch. Verified against this branch on React 17.0.2 and 16.14.0: two synchronous dispatches outside a batch (the request-callback path used by snackbarErrorHandler / snackbarSuccessHandler) both display; only a pair wrapped in unstable_batchedUpdates loses the first. Buffering at the Redux boundary changes the reducer contract that consumers also implement (summit-admin has its own base-reducer), so it belongs in a separate change. Leaving this open for the author to decide; I'd track it as a follow-up rather than fix it here.

[ako3131]

I agree, this fix can go in a follow-up PR.

[Copilot]

🔵 Needs a closer look

Unresolved queue handling can drop notifications during batched Redux dispatches or page transitions and leave old-page errors visible after navigation.

1 open issue

• Batched Redux notifications overwrite earlier messages

2 previously missed issues

In code that hasn't changed since last review

Page-change cleanup drops errors from the destination page

src/​components/​mui/​SnackbarNotification/​index.js:112

A destination page that calls errorMessage from a mount effect can lose its notification here. Child effects run before this ancestor's page-change effect, so the new error is appended behind the current message and then removed by this filter, even though it belongs to the destination page. Please associate messages with their originating page and remove only errors from the page being left, or prune the queue synchronously in the navigation handler before destination effects run.

Navigation leaves unopened errors pinned to the destination page

src/​components/​mui/​SnackbarNotification/​index.js:116

If errorMessage(...) and navigation occur in the same batched event while the snackbar is closed, this render has an error in current but still has open === false. The opening effect above schedules setOpen(true), while this guard skips closing it, leaving the old-page error pinned on the destination page. Handle unopened error heads during navigation too, rather than relying only on open, and cover enqueueing plus changePage in a single act.

2 resolved since last review

• Queued successes close immediately when promoted
• New errors are queued behind waiting successes

_{Review effort: Balanced}

[santipalenque]

@ako3131 two general things:

1. Grouping/stacking: in the thread Rocío proposed grouping errors into one snackbar ("3 errors · View details", with Copy copying all of them in order), or stacking as a fallback. This
   PR uses a plain queue instead, so with 3 errors the user has to close them one by one, and the waiting ones are dropped on navigation without ever being seen. Was that an intentional call
   for this iteration? If so, can we confirm with Rocío that it's OK, or plan grouping as a follow-up?

2. The copy feature is on feature/copyable-error-messages, which branches off main, not off this branch. Since errors only stay on screen so users have time to copy them, the two branches
   depend on each other, and they touch the same files (SnackbarNotification, its tests, constants.js). Should the copy branch be rebased onto this one, so they get reviewed and merged in
   order and we don't have to resolve conflicts later?

[santipalenque]

@ako3131 there is an issue here, if user doesn't close the error message and success messages start to stack up, then 15min later the user closes the error message (or navigates away) , a 15min old success message appears which would be very confusing. But at the same time there are many cases where we dispatch a success message and then navigate to a grid, so in that scenario we do want to show the success message.

I think we need to have one queue per message type, or at least one for errors and one for the rest, since the rest autohide and will eventually cleanup the queue by itself, and the errors will not. That means an error and a success could be on screen at the same time, so they'd need to stack instead of overlapping at the top center.

Another solution could be having an EXPIRATION_TIME for messages, so we avoid showing too old messages that can get confusing.

Happy to discuss or if you find a better solution go for it.

[santipalenque]

Rocío suggested ignoring click-away for all types, not only errors. Should we drop the !isSuccess here so successes don't close on an accidental click either?

[santipalenque]

@ako3131 thanks for the updates, nice work! Everything from my inline comments is in: the separate queues for errors and successes (success on top, error at the bottom), the longer
success timeout, and ignoring click-away for every type.

Two things are still open:

1. Grouping errors. Rocío's last suggestion in the thread was:

• One snackbar that shows the latest error plus a count: "3 errors · View details"
• "Copy details" copies every error in the group, in order and with timestamps, so support sees the full sequence
• Success/info keep auto-hiding and don't affect the error group
• Navigating away still clears the group (accepted trade-off, since the user can copy everything before leaving)
• If grouping is too much for this iteration, stacking as a fallback

The success/info part and the clear-on-navigation part already work with the separate queues 👍. But errors are still a plain queue, which is the case she wanted to avoid. Errors pile up
behind the one on screen, the user closes them one at a time, and on navigation the waiting ones are dropped without ever being shown or copied. Can we go with grouping here, or at least
stacking for now and grouping as a follow-up ticket?

2. Copy feature branch. Feature/copyable error messages #383 still targets main and is 24 commits behind this branch. Both touch SnackbarNotification, its tests and constants.js, and "Copy details" for a whole group
   needs both pieces together. Can we rebase feature/copyable-error-messages onto this branch so they get reviewed and merged in order?

Also, minor and maybe a follow-up: Copilot's point about batched Redux dispatches is still valid. baseState.snackbarMessage holds only one value, so dispatching an error and then a
success in the same handler loses the error before it reaches the queue.

[smarcet]

@ako3131 please review
@santipalenque for vis