Skip to content

Fix missing welcome panel after closing all documents#4107

Open
timon-schelling wants to merge 1 commit intomasterfrom
fix-close-all-docs
Open

Fix missing welcome panel after closing all documents#4107
timon-schelling wants to merge 1 commit intomasterfrom
fix-close-all-docs

Conversation

@timon-schelling
Copy link
Copy Markdown
Member

Closing all documents left the Welcome panel as the only tab, but the old active document index could still point to a tab that no longer existed. The fix is to reset it to 0 when the document list becomes empty.

Closing all documents left the Welcome panel as the only tab, but the old active document index could still point to a tab that no longer existed. The fix is to reset it to 0 when the document list becomes empty.
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the portfolio store to reset the active document index to zero when the document list becomes empty. The review feedback suggests a more robust approach by clamping the active document index within the valid bounds of the document list to prevent potential out-of-bounds errors during list updates.

subscriptions.subscribeFrontendMessage("UpdateOpenDocumentsList", (data) => {
update((state) => {
state.documents = data.openDocuments;
if (state.documents.length === 0) state.activeDocumentIndex = 0;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

While resetting the index to 0 when the list is empty fixes the reported issue, it is more robust to ensure the index is always within valid bounds whenever the document list changes. This prevents potential UI crashes or glitches if the active document is closed but other documents remain, causing the index to temporarily point out of bounds before the next UpdateActiveDocument message arrives.

Using Math.max(0, Math.min(..., length - 1)) correctly handles both the empty list case (resetting to 0) and the out-of-bounds case.

Suggested change
if (state.documents.length === 0) state.activeDocumentIndex = 0;
state.activeDocumentIndex = Math.max(0, Math.min(state.activeDocumentIndex, state.documents.length - 1));

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

No issues found across 1 file

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

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