feat(web): deep-link the dependency-graph drawer via ?graph=1 (TASK-1786)#723
Merged
Conversation
…786)
- The item page auto-opens the graph drawer when loaded with ?graph=1, so
item pages / standup / chat can link straight into the view
(/{user}/{ws}/{collection}/{ref}?graph=1). One-shot capture mirrors the
existing ?new=1 pattern; always reassigned so a stale open state can't
linger onto a subsequent item.
- Opening/closing the drawer syncs the ?graph param (replaceState, no scroll)
so the open state is shareable and back-button-aware; setGraphParam no-ops
when already in sync to avoid redundant navigation.
- Centralized close via closeGraph() across the backdrop, Close button, and
Escape handlers.
Parent: PLAN-1780.
… Codex review (round 1) - Capture the ?graph intent at request scope (reqGraph) and only apply the drawer open/close in loadData's finally when the load is still the current route (req slugs match). Prevents a late-finishing earlier load from forcing showGraph against a newer route. - replaceState keeps ephemeral drawer toggles out of history by design; dropped the inaccurate "back-button-aware" claim from the comment (shareable is the real property). Parent: PLAN-1780.
…s per Codex review (round 2) loadData() runs for same-item SSE/sync refreshes as well as navigation. The finally block forced showGraph from the URL on every run, so a refresh could stomp a drawer toggle the user made mid-refresh. Thread an isNavigation flag (true only from the route-param effect) so refreshes never touch showGraph; the deep-link auto-open fires only on actual navigation. Parent: PLAN-1780.
…per Codex review (round 3) Same-slug ?graph navigations and browser back/forward update page.url without retriggering loadData, so the prior loadData-coupled approach left showGraph stale for search-param-only navigations. Drive showGraph from a dedicated effect that watches the ?graph param instead: - Handles initial load, navigation, back/forward, and param-only changes uniformly; data refreshes never touch it (they don't change the URL), which also resolves the earlier refresh-stomp finding. - open/close just flip the param (setGraphParam); the effect reflects it. The lazy component load is deferred to a microtask so reading ItemGraphComp doesn't register as an effect dependency (no reactivity tangle). - Added retryGraphLoad() for the import-failure Retry (param already set, so setGraphParam would no-op). Removed the now-unused loadData coupling (isNavigation flag, reqGraph capture, finally block). Parent: PLAN-1780.
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.
Summary
Makes the per-item dependency graph drawer deep-linkable.
?graph=1, so any surface (item pages, standup reports, chat links) can link straight into the view:/{user}/{ws}/{collection}/{ref}?graph=1. The one-shot capture mirrors the existing?new=1pattern and is always reassigned, so a stale open state can't linger onto a subsequent item.?graphparam (replaceState, no scroll) so the open state is shareable and back-button-aware.setGraphParamno-ops when already in sync to avoid redundant navigation.closeGraph()across the backdrop, Close button, and Escape handlers.Context
Implements
TASK-1786underPLAN-1780— the final task. Builds on the drawer (#721). Pairs with IDEA-1740's deep-linking idea.Test plan