Keep backlink/scope-root ancestors expanded via force-open ids#329
Keep backlink/scope-root ancestors expanded via force-open ids#329Stvad wants to merge 5 commits into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dc1b918262
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| scrollWriteTimerRef.current = setTimeout(flushScrollTop, SCROLL_WRITE_DELAY_MS) | ||
| }, [flushScrollTop]) | ||
|
|
||
| const forceOpenBlockIds = useMemo<string[]>(() => { |
There was a problem hiding this comment.
Use a readonly memo type for force-open ids
This annotation makes the commit fail to compile: buildForceOpenBlockIds is declared as returning readonly string[], but useMemo<string[]> requires a mutable array, and yarn exec tsc -b --pretty false reports TS2345 here. Since yarn run check runs compile first, this blocks the app until the memo is typed as readonly string[] or the explicit generic is removed.
Useful? React with 👍 / 👎.
| ): Promise<Block | null> => { | ||
| if (current.id === scopeRootId) return null | ||
| const repo = current.repo | ||
| const forceOpen = toForceOpenSet(forceOpenBlockIds) |
There was a problem hiding this comment.
Remove the unused force-open set
This new local is never read, so the repo's compile step fails with TS6133 ('forceOpen' is declared but its value is never read) before tests or lint can run. Removing it, or actually using it in this function, is necessary for yarn run check to get past compilation.
Useful? React with 👍 / 👎.
| // is always a concrete block id (the card, or a promoted ancestor). | ||
| const surfaceId = shownId | ||
| const showingCard = surfaceId === currentId | ||
| const forceOpenBlockIds = useMemo( |
There was a problem hiding this comment.
Move the force-open memo above the early returns
This hook is called only on renders that make it past the queue === null and currentId === null returns above. After a card has rendered, finishing the last review card or restarting the session skips this useMemo, so React sees fewer hooks than the previous render and throws instead of showing the completion/empty state; compute this unconditionally before the returns or avoid a hook here.
Useful? React with 👍 / 👎.
Why
This fix keeps relevant ancestor blocks expanded in constrained surfaces (like backlinks and scope-root views) so the anchor block remains visible when a collapsed ancestor exists.
What changed
DefaultBlockRendererso specific block IDs can be rendered as expanded for backlinks/surface rendering.force-openmechanism.Notes
The behavior is kept scoped to the active surface so standard tree rendering elsewhere keeps existing collapse/expand semantics.