Repository navigation
Commit 2c876f0
fix(app-shell): the inbox opens on the first tab with items (objectui#11698) (#11743)
Fixes #11698
Clause-②: no
## What changed
`@object-ui/app-shell` `InboxPopover`: the bell's popover no longer
always opens on Notifications. The tab was a `useState('notifications')`
that nothing moved, while the badge above it counts `unreadTopics +
pendingApprovalsCount`. With 0 unread notifications and 3 pending
approvals the bell read "3" and the click showed "You're all caught up".
On every open the popover now picks its tab, in this order:
1. the tab the user last picked in this browser-tab session, if that tab
has something in it;
2. otherwise the first tab that has something in it: Notifications, then
Approvals, then Activity;
3. otherwise the user's last pick;
4. otherwise Notifications.
"Has something in it" means what each tab shows by default: an unread
notification topic for Notifications (the Unread filter is the default,
and unread topics are the badge's first addend), a pending approval for
Approvals (the badge's second addend), and an activity row for Activity.
The pick happens in the popover's open handler (`handleOpenChange`),
never while it is open, so a count that changes under an open popover
does not move the tab. Only a tab the user selects (`handleTabChange`)
is remembered. The tab the popover picks for them on open is not
remembered.
No new export, no new locale key, no new prop. `InboxPopover` is not
exported from the package. The other caller, the `global:notifications`
page block, gets the same behaviour.
## "Keeps the user's choice within a session": the definition pinned
here
Once the user selects a tab, every later open of an inbox bell in the
same browser tab reopens on it, across navigation and reload. A new
browser tab starts from the opening pick again. The pick is held in
`sessionStorage` (key `inbox-popover-tab`). Nothing goes to localStorage
or to the server.
Why not component state: the header bell is a different React instance
on Home, Organizations, an organization's pages, the AI page and inside
an app. `AppHeader` is mounted separately by `HomeLayout`,
`OrganizationsLayout`, `OrganizationLayout`, `AiChatPage` and
`ConsoleLayout`, so component state would forget the pick the first time
the user moved between Home and an app. `sessionStorage` is the scope
the ChatDock's expanded flag already uses for the same kind of per-tab
UI posture (`chatDockState.ts`). Every storage touch is guarded, so
private mode degrades to "nothing picked".
This survives a reload, which goes further than the dispatch's example
("until reload"). The reason is above. If the seat prefers page
lifetime, the change is to hold the pick in a module variable instead.
## Two acceptance texts, one edge where they disagree
The triage grade says the popover "keeps the user's choice within a
session". The card's Expected says "'You're all caught up' is never the
first thing shown under a non-zero badge". They disagree in one case:
the user picked Notifications, later reads the notification elsewhere,
and 3 approvals are still pending. Keeping the pick would show "You're
all caught up" under a "3".
Step 1 above settles it: a pick is kept only while its tab has something
in it. Otherwise the first tab with items wins. If every tab is empty,
the pick is kept, since the badge is then 0. This keeps both texts true
everywhere they can both be true. It is pinned by the case named "a
picked tab that is now empty yields to the first tab with items".
## The dispatch's mechanism hypotheses, measured
- **H1 (constant initial tab; badge = unread topics + approvals; `Tabs
value` is `tab`).** Confirmed on `origin/main` `6be0f7a`.
- **H2 (the counts may load after the popover opens).** Falsified. The
popover fetches nothing. Its counts are props from `useInboxBell`. That
hook reads the shared polled feeds (`useSharedInboxFeed`,
`useSharedPendingApprovalsCount`), which start when the header mounts
and do not depend on the popover being open. The badge renders from
those same props, so the pick at open time reads the numbers the bell
showed when it was clicked. If the user opens before the first poll
lands, the badge is 0 too, and the popover opens on Notifications or
Activity. When counts arrive while the popover is open, the tab does not
move. That is pinned.
- **H3 ("within a session" is ambiguous).** Defined above and pinned.
## Tests
New suite
`packages/app-shell/src/layout/__tests__/InboxPopover.openingTab-11698.test.tsx`,
11 cases. It mocks nothing: the real Radix popover and tabs, the real
router and the real English i18n pack. The selected tab is read from
Radix's `aria-selected`. The cases:
- the card's pin: 0 notifications and 3 approvals, so the badge reads 3
and the popover opens on Approvals, with no "You're all caught up"
anywhere in the popover;
- 2 notifications and 3 approvals open on Notifications;
- activity only opens on Activity;
- notifications that are all read are not items;
- all empty, with nothing picked, opens on Notifications;
- the popover's own pick is not remembered;
- the tab does not move while the popover is open;
- a pick is kept on reopen;
- a pick survives a remounted bell;
- an empty pick yields to the first tab with items;
- with every tab empty, the pick is kept.
`InboxPopover.displayLocale-10668.test.tsx` changed in its test helper
only. It read "the Notifications panel" as whatever the bell opened on.
Its first case selects Activity, and that pick now persists in
`sessionStorage` into the en-US control, so the control read the
Activity panel twice: 1 of 2 timestamps, red. The helper now selects
Notifications explicitly before reading it. Its assertions are
unchanged.
**Reverse verification**, on committed `770c375`. A trap-guarded script
wrote the base version of `InboxPopover.tsx` over the fix. Landing was
proven on disk: `onOpenChange={setOpen}` was found once, and
`openingTab(` and `handleOpenChange` zero times. The new suite then went
red with 7 failed and 4 passed, the same split as the pre-fix run, and
the card's pin failed with `expected 'Notifications' to match
/approvals/i`. The file was restored with `git checkout HEAD`, and the
restore was proven by its blob hash equalling the HEAD blob and an empty
`git diff HEAD`. The 4 cases that pass on the base version are the ones
the old constant default already satisfied: 2 and 3 open on
Notifications, all empty with nothing picked opens on Notifications, a
reopen within one mount keeps a pick, and an all-empty pick is kept.
## Gates (all on `770c375`, run from the worktree root)
| Command | Exit | The gate's own verdict line |
|---|---|---|
| `pnpm exec vitest run --maxWorkers=2` over the narrowed set below (via
`os-verify-lock`) | 0 | `Test Files 56 passed (56)` · `Tests 414 passed
(414)` |
| `pnpm exec turbo run build --filter='@object-ui/app-shell^...'
--concurrency=2` (the dependency closure the type-check reads) | 0 |
`Tasks: 28 successful, 28 total` |
| `pnpm --filter @object-ui/app-shell type-check` (the script name
`type-check` is echoed; it chains `tsc --noEmit` and `tsc -p
tsconfig.test.json`, and the test project's file list includes both
touched suites) | 0 | zero `error TS` lines |
| `pnpm --filter @object-ui/console exec vite build` | 0 | the bundle is
written to `apps/console/dist/index.html` |
| `pnpm exec eslint --format json` over the 3 touched `.ts`/`.tsx` files
| 0 | 3 files, 0 errors, 0 warnings |
| `pnpm check:control-bytes` | 0 | `check-control-bytes: OK (scanned
7691 tracked text file(s); skipped 85 binary).` |
| `pnpm check:test-path-roots` | 0 | `check-test-path-roots: OK` |
| `pnpm check:changeset-claims` | 0 | `No pending changeset names a file
this change touches.` |
| `pnpm check:pending-changeset-literals` | 0 | `No test source names a
pending changeset.` |
| `pnpm check:vi-mock-specifiers` | 0 | `check-vi-mock-specifiers: OK` |
| `pnpm check:new-line-citations` | 0 | `VERDICT
new-cross-file-line-citations: 0 new citation(s)` |
| `node scripts/check-changeset-presence.mjs` | 0 | `3 source file(s) of
1 released package(s) changed, and this change declares 1 changeset(s)`
|
Narrowed, and declared as narrowed. The whole app-shell package would
hold the shared verify lock for about 30 minutes, so the vitest run
covers every test directory and file that mounts the popover. That is
every file under `packages/app-shell/src/layout/`, which holds all the
`InboxPopover*`, `AppHeader*` and `ConsoleLayout*` suites, plus
`global-page-blocks.render` (the `global:notifications` block),
`sharedInboxFeed.twoSurfaces`, `HomePage.approvalsTarget`,
`KeyboardShortcutsDialog.wiredOnly-11674` (it mounts `ConsoleLayout`)
and the console's `approvalsInboxComponentRef`. The set was chosen with
`git grep` for `InboxPopover`, `AppHeader` and `global:notifications`
over test files. Like any grep, it cannot see a test that mounts the
header through a path that names none of them. The full farm belongs to
CI.
ESLint was narrowed and proven. The population comes from
`eslint.config.js` itself: the touched `.ts`/`.tsx` files fall under its
`**/*.{ts,tsx}` blocks. The file count is 3, read from `--format json`,
with 0 errors and 0 warnings. The invariance holds because
`--print-config` shows empty `parserOptions` (no `project` or
`projectService`). Type-aware linting is off, so this diff cannot move a
verdict on any untouched file.
The console compiles `InboxPopover.tsx` through its
`@object-ui/app-shell` alias to `src/`. That compile was measured with
the same command CI's E2E job uses, `pnpm --filter @object-ui/console
exec vite build`. It exited 0 and wrote `apps/console/dist/index.html`.
The diff adds no import, so the bundle graph the console's build checks
read is unchanged.
## Acceptance notes
- `@object-ui/app-shell` patch changeset
`.changeset/11698-inbox-opens-on-items.md`. `node
scripts/check-changeset-presence.mjs`: 3 published source files of 1
released package, 1 changeset. Not governed: `check-governed-queue-guard
--test` over the 4 paths printed NOT GOVERNED.
- No out-of-scope findings filed.
Session: `https://claude.ai/code/session_01CGZy1BGCjdN5cXqL9cnvB8`
---
_Generated by [Claude
Code](https://claude.ai/code/session_01CGZy1BGCjdN5cXqL9cnvB8)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent d39ac2e commit 2c876f0
4 files changed
Lines changed: 324 additions & 7 deletions
File tree
- .changeset
- packages/app-shell/src/layout
- __tests__
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
46 | 46 | | |
47 | 47 | | |
48 | 48 | | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
49 | 104 | | |
50 | 105 | | |
51 | 106 | | |
| |||
79 | 134 | | |
80 | 135 | | |
81 | 136 | | |
82 | | - | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
83 | 140 | | |
84 | 141 | | |
85 | 142 | | |
| |||
120 | 177 | | |
121 | 178 | | |
122 | 179 | | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
123 | 206 | | |
124 | 207 | | |
125 | 208 | | |
| |||
241 | 324 | | |
242 | 325 | | |
243 | 326 | | |
244 | | - | |
| 327 | + | |
245 | 328 | | |
246 | 329 | | |
247 | 330 | | |
| |||
312 | 395 | | |
313 | 396 | | |
314 | 397 | | |
315 | | - | |
| 398 | + | |
316 | 399 | | |
317 | 400 | | |
318 | 401 | | |
| |||
Lines changed: 10 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
83 | 83 | | |
84 | 84 | | |
85 | 85 | | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
86 | 95 | | |
87 | | - | |
88 | | - | |
89 | | - | |
90 | | - | |
| 96 | + | |
91 | 97 | | |
92 | 98 | | |
93 | 99 | | |
| |||
Lines changed: 219 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
0 commit comments