Repository navigation
fix(announcements): pop the full-screen announcement from unread state, not a per-browser watermark - #1285
fix(announcements): pop the full-screen announcement from unread state, not a per-browser watermark#1285JamieRuderman wants to merge 10 commits into
Conversation
…of gating on a per-browser watermark
…red date constants
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08026ea8f4
ℹ️ 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".
| // Marking it read still happens on dismiss to clear the navigation badge. | ||
| announcements.setPresentedThrough(latestUnread.modified?.getTime() || 0) | ||
| }, [activeId, announcements, dismissedIds, latestUnread?.id, presentedThrough]) | ||
| setPresentedId(presentable.id) |
There was a problem hiding this comment.
When notice A has been presented but its read request fails, presenting a newer notice B overwrites this sole guard. If B is later omitted by a refresh—for example because it expires, is disabled, or is deleted—selectPresentableAnnouncement selects unread A again and the dialog reopens it during the same session. Preserve all IDs presented during the mount rather than only the most recent one.
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
…stamped when a notice is enabled
|
@codex review |
… its start is blank, so no backfill is needed
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c82d81daea
ℹ️ 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".
| // Marking it read still happens on dismiss to clear the navigation badge. | ||
| announcements.setPresentedThrough(latestUnread.modified?.getTime() || 0) | ||
| }, [activeId, announcements, dismissedIds, latestUnread?.id, presentedThrough]) | ||
| setPresentedIds(ids => [...ids, presentable.id]) |
There was a problem hiding this comment.
When “Show latest announcement” previews an unread notice that has not already been auto-presented in this mount, only the normal presentation path adds its ID here. Closing the test dialog deliberately skips announcements.read, then handleExited clears activeId, so this effect immediately opens the same notice again as a normal announcement; closing that second dialog marks it read, contrary to the test control’s promise not to change read status. Add test-presented IDs to the same guard (as the removed dismissedIds update did).
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
This branch has not been deployed
1 participant
Problem
The full-screen announcement popup was gated by a
presentedThroughwatermark stored in the persistedannouncementsslice. That slice is shared by every account on a browser, and switching saved accounts reloads without purging it. So once one account saw a notice, every other account on that browser skipped it, even though their per-account read state said unread. A fresh sign-in also seeded the watermark to "now", which suppressed any notice published before that sign-in.Seen in practice: a notice published as jamie@remote.it never popped for a test account signed in afterwards on the same browser.
Change
The popup is now driven by per-account unread state from the API.
Eligibility: only the newest non-banner notice can pop, and only while it is unread and was saved on or after 2026-07-20, the date the full-screen popup was introduced (Add full-screen announcement presentation #1129). Older notices stay in the list but never pop, so nobody is walked through a backlog.
Fresh data only: the dialog waits for this session's notices fetch, tracked by a non-persisted
ui.announcementsFetchedflag. Before that, the persisted list may belong to the previous account.No reopen: the dialog remembers the notice it presented, so a failed read call cannot reopen it the moment it closes. A newer notice published mid-session still pops on the next fetch.
Stale responses: a notices response that lands after its account signed out is dropped.
Chat popout: the dialog no longer mounts in the popout window, which previously relied on the watermark to stay quiet.
The watermark, its reducers, and its reset in "Clear viewed announcements" are removed.
Start date is the announcement's date: the popup, the choice of newest notice, the list order and the card's date all use the notice's From date, falling back to the last save while it's blank. Editing a notice no longer moves its date or re-announces it.
Admin form fills it in: a blank From is filled on save. Enabling a notice sets it to now. Saving a notice that was already enabled sets it to that notice's last save before this one, so editing it doesn't re-announce it. Drafts stay blank until they're enabled.
No backfill
Existing notices keep a blank From, and the app falls back to their last save, which hasn't moved. Each one gets its real date the first time it's edited through the admin form. A bulk update through
updateNoticewould have reset every notice'smodified, which released app versions still use for the card date and the popup.Behavior to know
Testing
npm test -w=frontendandnpm run typecheckpass.