feat(nav): add AMP and Geothermal sidebar groups - #363
Open
jirhiker wants to merge 6 commits into
Open
Conversation
This is the half of "drop the Sandbox concept" that never landed. The original commit was titled "drop the Sandbox concept for a Geothermal nav group" and did both; an amend before PR #358 kept only the removal, so the geothermal grids lost their Sandbox entry without gaining the replacement. They have been reachable by URL only ever since. - Add GeothermalNavItem: a top-level Geothermal group holding Records, Inventory, and Temp-Depth. - Add SHOW_WIP_NAV, gated on dev or preview only, matching the existing gate in recordsGridLogic.ts. Staging and production do not show it. - Update AGENTS.md and the well-inventory doc, which still described nav placement as pending. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
jirhiker
force-pushed
the
feat/geothermal-sidebar-group
branch
from
August 27, 2026 14:59
bacc598 to
60568a9
Compare
Preview DeploymentPreview URL: https://preview-feat-geothermal-sidebar-group-auejgdbofq-uc.a.run.app |
Member
Author
|
@lizalino ☝️ |
The sidebar listed nine peers with no sense of which domain each belonged to. Wells, Field Sheets and Contacts are the AMP surfaces, so they move under an AMP group, and the geothermal group follows directly below it. ResourceNavItem needed two changes to support a grouping entry: - A group is now active for its own href *or* any visible child's. Matching only the parent href collapsed the group when moving to a child that does not nest under it — Contacts, at /ocotillo/contact, is not under /ocotillo/well. - A parent may omit `resource`. AMP is a grouping entry, not a resource, and gating it on one of its children's would hide the whole group from anyone who cannot see that child. Each child still gates on its own, the way PRIMARY_NAV already allowed. AppShell anchors the geothermal group to AMP_NAV_ID rather than the label, so rewording "AMP" cannot silently move it.
Two bugs, both from the group's open state being driven by the route. A group could not be collapsed while you were inside it: `isOpen` was `sectionActive || open`, and the toggle handler ignored the change while the section was active. Nothing hit this before, because no RESOURCE_NAV entry had children — AMP covers the pages you use most, so it was pinned open in practice. Groups also behaved like an accordion: opening one collapsed the other. Every group trigger is also a link, so clicking AMP navigated, which drove Geothermal's close-on-navigate-away, and vice versa. Extract useNavSectionOpen: navigating into a section opens it, and nothing but the user closes it. Both groups now share it, so GeothermalNavItem loses its own route-watching effect.
Both group headers were links, so expanding AMP jumped you to Wells and expanding Geothermal jumped you to the records grid. Opening a group to see what is in it should not move you off the page you are on. The header is now a plain toggle; switching page is the child link's job. AMP loses its href along with it — it is a grouping entry, not a destination, and the group still tracks the route through its children's hrefs. GeothermalNavItem no longer needs useNavigate.
Puts a separator below the Geothermal group, so AMP and Geothermal read as domain groups and Projects onward as the flat resource list. Wrapped in an li: SidebarMenu renders a ul, and SidebarSeparator renders a div, which is not a valid child of one.
Preview DeploymentPreview URL: https://preview-feat-geothermal-sidebar-group-auejgdbofq-uc.a.run.app |
Drops SHOW_WIP_NAV. The geothermal pages are no longer WIP surfaces to keep off staging, so the group renders like any other. AGENTS.md's WIP-gating example named SHOW_WIP_FEATURES, which never existed; point it at BYPASS_GEOTHERMAL_GATE in recordsGridLogic.ts, which is now the only gate of that shape. The well-inventory doc no longer describes the group as dev/preview only.
Preview DeploymentPreview URL: https://preview-feat-geothermal-sidebar-group-auejgdbofq-uc.a.run.app |
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
1 participant
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.
Gives the sidebar two domain groups — AMP and Geothermal — and fixes the group behaviour that surfaced once anything actually used it.
Recovering the Geothermal group
1e7b505"drop the Sandbox concept" started life as "drop the Sandbox concept for a Geothermal nav group" and did both things. An amend before #358 kept only the removal, so the geothermal grids lost their Sandbox entry without gaining the replacement — reachable by URL only ever since.1e7b505's own message still says "a geothermal nav group lands separately". This is that.The group holds Records, Inventory, and Temp-Depth. All three already route on staging (
/geothermal/wells/records-grid,/inventory,/temp-depth); this only adds the way in.Grouping AMP
The sidebar listed nine peers with no sense of which domain each belonged to. Wells, Field Sheets, and Contacts are the AMP surfaces, so they move under an AMP group, with Geothermal directly below it and a divider separating both from the flat resource list:
Projects stays top-level — it is a wells sub-page (
/ocotillo/well/projects), so the AMP group does open while you are on it, and the active-item highlight still lands on Projects becauseactiveHreftakes the longest match.Fixes this needed
ResourceNavItemsupportedchildrenbut nothing had ever used it, so three bugs had never fired:A group could not be collapsed while you were inside it.
isOpenwassectionActive || openand the toggle handler ignored the change while active. AMP covers the pages you use most, so in practice it was pinned open.Groups behaved like an accordion. Both group headers were links, so clicking AMP navigated, which tripped Geothermal's close-on-navigate-away, and vice versa — opening one collapsed the other.
Expanding a group jumped you to a page. Opening AMP took you to Wells; opening Geothermal took you to the records grid. Opening a group to see what is in it should not move you off the page you are on.
The first two are now one extracted hook,
useNavSectionOpen: navigating into a section opens it, and nothing but the user closes it. Both groups share it. For the third, group headers are plain toggles — switching page is the child link's job — and AMP drops itshrefalong with it, since it is a grouping entry rather than a destination.Two smaller changes fell out:
resource. It wasresource!unconditionally. AMP is a grouping entry, not a resource, and gating it on one child's resource would hide the whole group from anyone who cannot see that child. Children still gate individually — the same shapePRIMARY_NAValready allowed./ocotillo/well.SHOW_WIP_NAV is gone
The recovered commit gated the group on a dev-or-preview flag. The geothermal pages are no longer WIP surfaces to keep off staging, so the group renders like any other and the flag is removed.
AGENTS.md's WIP-gating example namedSHOW_WIP_FEATURES, which never existed in the codebase. It now points atBYPASS_GEOTHERMAL_GATEinrecordsGridLogic.ts, the only remaining gate of that shape.Reading the diff
AppShell.tsxshows ~640 changed lines but only ~310 undergit diff -w. The rest is re-indentation from unwrapping<CanAccess>around two large JSX blocks — no reformatting of unrelated code. Bothstagingand this branch are clean under the repo's Biome config;git diff -wis the honest view.Verification
tsc --noEmitcleanvite buildclean — only the pre-existing chunk-size noticevitest runwith the prism mock server up: 52 files, 433 passed, 1 skipped, 0 faileduseNavSectionOpen— including the two behaviours reported here, collapsing while the section is active and staying open when navigation leavesbiome lint— the 3 remainingAppShell.tsxwarnings are pre-existing onstaging, confirmed by stashingNot verified in a browser: the app is behind OIDC and I had no session to render the sidebar with. The nav behaviour above was reported from a running app and re-checked against the hook's tests; a preview deploy is still the place to click through it.
🤖 Generated with Claude Code