Skip to content

Keep backlink/scope-root ancestors expanded via force-open ids#329

Open
Stvad wants to merge 5 commits into
masterfrom
codex/backlink-force-open-surface
Open

Keep backlink/scope-root ancestors expanded via force-open ids#329
Stvad wants to merge 5 commits into
masterfrom
codex/backlink-force-open-surface

Conversation

@Stvad

@Stvad Stvad commented Jul 6, 2026

Copy link
Copy Markdown
Owner

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

  • Threaded a surface-scoped list of forced-open block IDs through selection and shortcut activation state.
  • Added a centralized render hint in DefaultBlockRenderer so specific block IDs can be rendered as expanded for backlinks/surface rendering.
  • Removed duplicate/parallel top-level special-casing by consolidating behavior around the shared force-open mechanism.
  • Updated affected tests to cover propagation of the new surface hint.

Notes

The behavior is kept scoped to the active surface so standard tree rendering elsewhere keeps existing collapse/expand semantics.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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[]>(() => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Comment thread src/utils/selection.ts
): Promise<Block | null> => {
if (current.id === scopeRootId) return null
const repo = current.repo
const forceOpen = toForceOpenSet(forceOpenBlockIds)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant