Skip to content

open by default custom store variables#649

Merged
lazarusA merged 5 commits into
mainfrom
la/open_store
May 18, 2026
Merged

open by default custom store variables#649
lazarusA merged 5 commits into
mainfrom
la/open_store

Conversation

@lazarusA
Copy link
Copy Markdown
Member

Copy link
Copy Markdown

@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 introduces a mechanism to automatically open the variables panel upon initializing a remote store by adding a shouldOpenVariablesAfterInit flag to the global state. It also refactors LandingHome.tsx to improve metadata fetching with race condition checks and updated dependency arrays. Feedback indicates that including shouldOpenVariablesAfterInit in the useEffect dependency array causes redundant network requests, and it is recommended to access this state directly from the store within the promise callback to optimize performance.

Comment thread src/components/LandingHome.tsx Outdated
@lazarusA
Copy link
Copy Markdown
Member Author

/gemini review

Copy link
Copy Markdown

@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 introduces a storeFromURL flag to the global state to automatically open the variables panel when a dataset is loaded via a URL parameter. The changes span the global store definition, the store initializer, and the main landing component. The review feedback identifies several opportunities to improve the robustness of this feature, specifically by ensuring the storeFromURL flag is reset after its initial consumption to prevent the panel from re-opening during re-renders. Additionally, the feedback points out a potential bug in how local stores are identified and suggests optimizing state access by using variables already available in the component scope rather than calling getState() inside effects.

Comment thread src/components/LandingHome.tsx
Comment thread src/components/LandingHome.tsx Outdated
Comment thread src/components/LandingHome.tsx Outdated
Comment thread src/components/LandingHome.tsx Outdated
@lazarusA lazarusA merged commit a9011b2 into main May 18, 2026
6 checks passed
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.

Local Zarr Server needs manual refresh

1 participant