Fix experimental stacks feature accidentally leaking into the ethui#1495
Fix experimental stacks feature accidentally leaking into the ethui#1495
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds conditional rendering for the "Local Stacks" feature based on a runtime check. The changes introduce a new backend call (is_stacks_enabled) to determine if the Stacks feature is available, and conditionally shows/hides UI elements accordingly.
Key changes:
- Added
useInvokehook calls to check if Stacks is enabled viais_stacks_enabledbackend command - Wrapped "Enable Stacks" settings toggle in conditional rendering
- Wrapped the entire "Local Stacks" section in the networks page with conditional rendering
- Added
enabledoption to the runtime state query to prevent unnecessary calls when Stacks is disabled
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| gui/src/routes/home/_l/settings/_l/general.tsx | Conditionally renders the "Enable Stacks" toggle based on isStacksEnabled flag |
| gui/src/routes/home/_l/networks/_l/index.tsx | Conditionally renders the entire "Local Stacks" section and optimizes the runtime state query |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| refetchInterval: ({ data }: { data: { running: boolean } | null }) => | ||
| data?.running ? false : 1000, | ||
| refetchOnWindowFocus: false, | ||
| enabled: isStacksEnabled, |
There was a problem hiding this comment.
The enabled option will be undefined when isStacksEnabled is not yet loaded, which could cause the query to execute when it shouldn't. Change to enabled: isStacksEnabled === true or enabled: !!isStacksEnabled to ensure the query only runs when explicitly enabled.
| enabled: isStacksEnabled, | |
| enabled: isStacksEnabled === true, |
| ); | ||
|
|
||
| if (!networks || !settings || runtimeLoading) { | ||
| if (!networks || !settings || (isStacksEnabled && runtimeLoading)) { |
There was a problem hiding this comment.
The loading condition doesn't account for when isStacksEnabled is still loading (undefined). When isStacksEnabled is undefined, the component won't show a loading state but will proceed to render, potentially showing the Stacks section briefly before it loads. Consider adding a check for the loading state of isStacksEnabled query using isLoading from useInvoke.
| } | ||
| /> | ||
| </div> | ||
| {isStacksEnabled && ( |
There was a problem hiding this comment.
When isStacksEnabled is undefined (during initial load), the switch will be hidden. This creates a flash where the UI element appears/disappears. Consider checking the loading state from useInvoke and showing a loading indicator, or default to showing the switch until the query completes.
2 participants
No description provided.