fix(home-v2): bookmark toolbar follow-up fixes - #354
Merged
Conversation
Addresses the four product-facing minors from the toolbar branch review (P-10 follow-ups): openDashboardPin rejects again on a failed open so the pinned-apps inline role=alert renders (instead of only the passive shell notice); toolbar open/context-action failures surface through a new onActionError prop threaded to every ToolbarLink (they previously vanished in catch(() => undefined)); the page viewport sizes against a measured --v2-chrome-height set by a ResizeObserver on the browser chrome, replacing the hardcoded 98px/140px offsets that assumed default text size and a fixed-height toolbar; and the toolbar visibility row in Appearance renders as a disabled loading row until the collections snapshot arrives instead of appearing late. Parity review P-10 follow-ups (projects/qortium-home-2.1/parity-review-2026-08-24). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The four product-facing minors the adversarial review found on the bookmarks toolbar (#345), now that it's merged:
openDashboardPinswallowed failures into the passive shell notice, soHomeV2PinnedApps'role=alertpath (which renders the error next to the pin viarunAction) never fired. It rejects again with the failure message; the context-menu "open" path reaches the same inline surface.catch(() => undefined). A new optionalonActionErrorprop threads throughToolbarItems/FolderMenuItems/ToolbarLink; the live app wires it to the shell notice (the toolbar has no inline alert surface of its own).calc(100vh - 98px)(and- 140pxwith the toolbar via:has), which assumed the default text size and a fixed 42px toolbar; after the typography PR the chrome height genuinely varies. AResizeObserveron the browser chrome now sets--v2-chrome-heighton the shell root and both viewport rules use it (98px fallback for environments without observers or before mount). The:hasspecial case is deleted.Testing
tscclean;test:home-v2-foundation,test:home-v2-browser-chrome,test:home-v2-bookmark-toolbarpass;build:renderersucceeds. Worth a quick packaged check that the app view bottom edge is correct with toolbar shown/hidden at Medium and Huge text sizes (the smokesmoke:desktop:home-v2-bookmark-toolbaralso covers the toolbar flows in CI-less form if you want it locally).🤖 Generated with Claude Code