Fix hidden toast content for front-most filtered stack item - #55
Conversation
- keep the front-most visible toast readable when thread filtering hides others - extract visibility rule into `toast.logic.ts` - add unit tests for collapsed-content visibility behavior
|
No actionable comments were generated in the recent review. 🎉 WalkthroughAdds Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Hide collapsed content for non-front toasts in stacks while keeping the front-most toast readable in
|
Greptile SummaryFixes a bug where thread filtering caused the front-most visible toast's content to be hidden in collapsed stacks. The root cause was that Base UI's
Confidence Score: 4/5
Important Files Changed
Flowchartflowchart TD
A[All toasts from Base UI] -->|filter by activeThreadId| B[visibleToasts array]
B --> C{Iterate with visibleIndex}
C --> D[visibleIndex === 0]
C --> E[visibleIndex > 0]
D --> F["shouldHideCollapsedToastContent = false\n(front toast content visible)"]
E --> G["shouldHideCollapsedToastContent = true\n(content hidden unless expanded)"]
F --> H[Toast.Content rendered with opacity:100]
G --> I["Toast.Content rendered with\nnot-data-expanded:opacity-0"]
H --> J[User sees front toast content]
I -->|User hovers/expands stack| K[data-expanded:opacity-100 overrides]
Last reviewed commit: 0beaf09 |
- Add position-specific start transform for top-right toasts - Apply matching top-right end transform when dismissing without swipe
Two tasks in one commit because both edit ChatView.tsx and cannot be separated by pathspec. DIFF AS A DOCK PANEL (pingdotgg#56). Part B of our own frozen spec, deferred since Part A shipped. The app had TWO tab systems on screen -- the dockview grid, and a second tab set inside the single chat panel. Diff moves first because it already resolved its own thread identity from the URL rather than taking it from ChatView's scope; the other three take a dozen callbacks each and will need rewriting. The structural problem worth recording: ChatView is a DESCENDANT of the dock (ChatDock -> ChatPanel -> ChatView), so the components that need to open a panel cannot receive a ref to the thing containing them. Solved with a module-scope handle registered on mount and cleared on unmount, matching the two singletons already in that file. It warns loudly rather than no-opping silently when null. THE MIGRATION IS PROVEN COMPLETE, NOT ASSERTED. A review found Cmd/Ctrl+D still driving the old store -- and because the render case had been correctly removed while the tab strip still knew how to draw a Diff tab, the shortcut produced a VISIBLE TAB THAT RENDERED NOTHING. Third instance of "looks wired, does nothing" in this repo. The fix was not to patch two call sites. `"diff"` is gone from RightPanelKind and RightPanelSurface, so the compiler enumerated every straggler: exactly six, across two files, all genuinely dead. This is the template for Files, Terminal and Browser -- move the surface, delete its kind, and completeness becomes a build error rather than something a human eyeballed. The one place that must still recognise the retired kind is the persistence migration, which casts past the union with a comment explaining that handling data older than the current type is its entire job. Cmd+D also lost its toggle in the first pass -- open-and-focus with no close. The lane flagged it rather than letting me find it. Not accepted: the keybinding is named `diff.toggle`, the handler is named onToggleDiff, and "use the tab x" defeats the point of a keyboard shortcut. Now a genuine toggle, with the CLOSE half specifically mutation-proven, because a toggle that only ever opens is the exact bug being fixed. The layout-persistence risk was checked rather than assumed: `persist()` stamps knownPanelIds from the current catalog on every save, so a close survives reload as "closed on purpose" instead of being re-grafted. Storage v7->8 strips persisted "diff" surfaces, and recomputes `isOpen` from surviving surfaces -- without that, a user whose ONLY surface was diff resumed into an empty right panel they never asked for. Invisible to the first test because its fixture always had a second surface. THREE.JS PLAY (pingdotgg#51). `autoOpenPreview` has been declared in contracts, collected by the settings form, and persisted to disk since it was written -- and nothing ever read it. A user could tick the box, save, and nothing happened. It now opens the preview when a script with a previewUrl starts. It waits for the port scanner to report a listener attributed BY PID to the exact terminal the script ran in, then opens the CONFIGURED url rather than the scanner's reported one -- those differ when a port gets bumped, and the user configured what they configured. Guarded on desktop before anything starts, so in a browser it genuinely does nothing rather than failing gracefully. Its mutation proof is the right shape: deleting the open call while still returning `opened: true` turns the test red with zero calls to the mock -- the exact "looks wired, does nothing" shape this task exists to fix, reproduced deliberately to prove the test catches it. Full typecheck, npm test and fmt:check green. Live browser E2E is an OPEN GATE, tracked for pingdotgg#55 -- the dock is half migrated, so a live pass today would measure a transitional state.
…gdotgg#58) * feat(settings): build the Secrets administration page in T3 Replace the Secrets NotInT3Yet placeholder with a real panel: a Global and a Personal section, each listing its scope's secrets with an enable switch, edit, and delete, plus a create/edit dialog. Reads go through the shared moatless query helpers keyed under `secrets/{scope}`, so a write invalidates both scopes together. The row-derivation logic (kind labels, active-first ordering) lives in a pure `secretRows.ts` with co-located tests; the panel only reads atoms and renders T3's own settings primitives. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(settings): build the Users administration page in T3 Replace the Users NotInT3Yet placeholder with a list and a detail page. The list shows everyone with an account — monogram, name, admin and bot tags — and leads to one user's page. The detail page edits name, email, and the global role with dirty tracking and an explicit save, and shows the read-only account identity beneath it. There is no single-user read on the wire, so the detail page reads the same list the index does and finds its login in it; the two share the `users` key so a role change refetches both at once. Row derivation (display name, monogram, humans-before-bots ordering) is a pure `userRows.ts` with co-located tests. The detail route uses the trailing-underscore file form so it does not nest under the list route, and is recorded in the merge inventory. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(settings): build the Moatless Loops administration pages in T3 Add the Loops list and Loop detail settings surfaces, replacing the NotInT3Yet placeholders. The list shows each loop's state, git provenance and source summary, and links to a detail page that edits the loop's general fields, configuration and schedule with the workspace SaveBar pattern, exposes lifecycle actions (pause/resume/approve-and-activate), git override/restore, and delete. Source derivation and provenance live in a pure `loopRows.ts` module with co-located tests. Detail route uses the underscore form to avoid nested-route rendering. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(settings): build the Moatless Integrations administration page in T3 Replace the NotInT3Yet placeholder on /settings/integrations with a panel of three sections: Connections (adapter connections a loop can subscribe to — listed, created, and opened to a detail page that removes them), Apps (the configured adapter apps and which of each app's secrets are set, read-only), and GitHub (registered GitHub apps, their installations and the default installation, read-only). Naming, ordering and secret fingerprint derivation live in a pure integrationRows.ts module with co-located tests. Connection detail route uses the underscore form to avoid nested-route rendering. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(settings): administer skills plugins and their activation in T3 Adds the Skills administration surface: a plugin catalog and, per plugin, the skills it sources with a control for the deployment-wide default and the viewer's own override, side by side. The delivered badge is read straight from /plugins/effective rather than recomputed from the two controls — off-for-everyone-plus-on-for-you is the server's call to resolve, so a skill can read "Not set / Not set" and still be Delivered because its plugin is on. Precedence lives in skillRows.ts behind a test that pins the source-off-with-skill-on and unset-versus-off cases. Detail route uses the underscore form so it does not nest under the list panel. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(spec): mark moatless-admin spec in review and correct stale claims All five administration surfaces are now built and under review as PRs pingdotgg#51–pingdotgg#55, so the spec no longer reads "in progress". Records the merge order the shared queries.ts forces, and the two follow-ups that land once the five merge (delete NotInT3Yet, move this spec to .plans/completed/). Also fixes two claims that drifted from what shipped: the acceptance criterion still said the not-built placeholder links into the SPA, which the placeholder decision earlier in the same doc deliberately reversed; and two seam-table module paths pointed at apps/web/src/moatless/ rather than where the pure modules actually landed under components/settings/moatless/. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(spec): unbreak the surfaces table split by the route-convention note The trailing-underscore route-convention paragraph sat between two table rows, so Markdown ended the table at it and rendered every row below — loops through users, ten of the twelve surfaces — as literal pipes. Move the paragraph below the last row so the table stays one block. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(settings): retire the NotInT3Yet placeholder and archive the spec Every administration surface now has a real panel, so no route renders the placeholder. Its own comment said to delete the file when the last usage went; this is that moment. The merge-inventory row stops describing a build where some entries lead to a placeholder. Neither of these could be done while the surfaces were five separate branches — each still needed the placeholder for the other four. Combining them into one change is what makes both possible, so they land here. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(fork): list the merge inventory as entries, not tables The inventory conflicted on every branch that touched it, and almost none of that was disagreement. The formatter re-pads a Markdown table across all of its rows when one cell changes width, so adding a single route path to the administration-pages row rewrote all 60 lines of the block; two branches editing two unrelated entries then conflicted on every line between them. Measured on this file, adding one route path to the administration pages: 60 changed lines as a table, 1 as a list. So the tables are gone. Every section is a list of entries: a title line, an optional nested `Paths:` list with one path per line, and the rule. The formatter leaves list items and prose alone — verified by running `vp fmt` over the result and getting a byte-identical file back. The header paragraph says so, so this does not get turned back into a table. Content is carried over unchanged. Verified by extracting every backticked path from both versions: 117 before, 117 after, none lost and none invented. Three deliberate edits beside the reformat: - "the three fork plans above" now says four; that entry has listed four plans since the administration spec was archived. - "row" means a table row, and there are no tables now, so policy entries and the skill's vocabulary say "entry" — including the `decide, then add an entry` policy value the skill defines. - `.agents/skills/fork-upstream-merge/**` gets a path-policy entry. It is fork-authored and was in no entry at all; upstream owns the other four skills under `.agents/skills/` and adds more, so a blanket `ours` there would be wrong. Checked against a shallow `upstream/main` fetch, which also confirmed the two counts this file quotes are still current: 32 of 36 `.plans/` files and 34 of 39 under `apps/web/src/browser/`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…ABLED (pingdotgg#55) Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Summary
visibleToastsfirst and bases collapsed-content hiding on visible stack position, not Base UIdata-behindstate.shouldHideCollapsedToastContentfor clearer behavior and easier maintenance.Testing
apps/web/src/components/ui/toast.logic.test.tscovering:Summary by CodeRabbit
New Features
Tests