You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "YousufFFFF (via GitHub)" <gi...@apache.org> on 2026/02/09 07:18:51 UTC

[PR] fix(explore): update metric and column arrow state on datasource change [superset]

YousufFFFF opened a new pull request, #37800:
URL: https://github.com/apache/superset/pull/37800

   <!---
   Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
   Example:
   fix(dashboard): load charts correctly
   -->
   
   ## Summary
   Fixes #37444 an issue in the Explore datasource panel where the expand/collapse arrow
   state for metrics and columns could become out of sync when the datasource
   folders update.
   
   This ensures the arrow state stays consistent with the underlying collapsed
   state after datasource changes.
   
   ## Testing
   - Verified locally in Explore view
   - Confirmed metric and column arrows update correctly on datasource change
   
   ## After Fixing:
   https://github.com/user-attachments/assets/d1644bc6-07c9-4b71-923c-48450e26c605
   
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(explore): update metric and column arrow state on datasource change [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #37800:
URL: https://github.com/apache/superset/pull/37800#discussion_r2781034299


##########
superset-frontend/src/explore/components/DatasourcePanel/DatasourceItems.tsx:
##########
@@ -105,6 +105,14 @@ export const DatasourceItems = ({
     ),
   );
 
+  useEffect(() => {
+    setCollapsedFolderIds(
+      new Set(
+        folders.filter(folder => folder.isCollapsed).map(folder => folder.id),
+      ),
+    );
+  }, [folders]);

Review Comment:
   <div>
   
   
   <div id="suggestion">
   <div id="issue"><b>State Reset Bug</b></div>
   <div id="fix">
   
   The added useEffect resets the collapsedFolderIds state whenever folders changes (e.g., during search), since folders.isCollapsed is always false, this uncollapses all folders unexpectedly, losing user toggles. Remove it to preserve local state.
   </div>
   
   
   <details>
   <summary>
   <b>Code suggestion</b>
   </summary>
   <blockquote>Check the AI-generated fix before applying</blockquote>
   <div id="code">
   
   
   ````suggestion
    
   ````
   
   </div>
   </details>
   
   
   
   </div>
   
   
   
   
   <small><i>Code Review Run #3f6a80</i></small>
   </div>
   
   ---
   Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
   - [ ] Yes, avoid them



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(explore): update metric and column arrow state on datasource change [superset]

Posted by "YousufFFFF (via GitHub)" <gi...@apache.org>.
YousufFFFF closed pull request #37800: fix(explore): update metric and column arrow state on datasource change
URL: https://github.com/apache/superset/pull/37800


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(explore): update metric and column arrow state on datasource change [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #37800:
URL: https://github.com/apache/superset/pull/37800#discussion_r2781073082


##########
superset-frontend/src/explore/components/DatasourcePanel/DatasourceItems.tsx:
##########
@@ -105,6 +105,14 @@ export const DatasourceItems = ({
     ),
   );
 
+  useEffect(() => {
+    setCollapsedFolderIds(
+      new Set(
+        folders.filter(folder => folder.isCollapsed).map(folder => folder.id),
+      ),
+    );
+  }, [folders]);

Review Comment:
   <!-- Bito Reply -->
   The updated logic initializes collapsed state once from folders on mount, preserving user toggles and syncing arrow state. Change the useEffect dependency to [] to avoid resetting on folders updates.
   
   **superset-frontend/src/explore/components/DatasourcePanel/DatasourceItems.tsx**
   ```
   useEffect(() => {
       setCollapsedFolderIds(
         new Set(
           folders.filter(folder => folder.isCollapsed).map(folder => folder.id),
         ),
       );
     }, []);
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(explore): update metric and column arrow state on datasource change [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #37800:
URL: https://github.com/apache/superset/pull/37800#issuecomment-3871144380

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at .
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(explore): update metric and column arrow state on datasource change [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #37800:
URL: https://github.com/apache/superset/pull/37800#discussion_r2781716836


##########
superset-frontend/src/explore/components/DatasourcePanel/DatasourceItems.tsx:
##########
@@ -105,6 +105,14 @@ export const DatasourceItems = ({
     ),
   );
 
+  useEffect(() => {
+    setCollapsedFolderIds(
+      new Set(
+        folders.filter(folder => folder.isCollapsed).map(folder => folder.id),
+      ),
+    );
+  }, [folders]);

Review Comment:
   <!-- Bito Reply -->
   This question isn’t related to the pull request. I can only help with questions about the PR’s code or comments.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(explore): update metric and column arrow state on datasource change [superset]

Posted by "YousufFFFF (via GitHub)" <gi...@apache.org>.
YousufFFFF commented on code in PR #37800:
URL: https://github.com/apache/superset/pull/37800#discussion_r2781714618


##########
superset-frontend/src/explore/components/DatasourcePanel/DatasourceItems.tsx:
##########
@@ -105,6 +105,14 @@ export const DatasourceItems = ({
     ),
   );
 
+  useEffect(() => {
+    setCollapsedFolderIds(
+      new Set(
+        folders.filter(folder => folder.isCollapsed).map(folder => folder.id),
+      ),
+    );
+  }, [folders]);

Review Comment:
   Thanks for the suggestion. The latest commit already ensures the collapsed state is initialized only once while guarding against early renders where `folders` may not yet be populated. Subsequent `folders` updates preserve user-controlled collapse state.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(explore): update metric and column arrow state on datasource change [superset]

Posted by "YousufFFFF (via GitHub)" <gi...@apache.org>.
YousufFFFF commented on code in PR #37800:
URL: https://github.com/apache/superset/pull/37800#discussion_r2781069091


##########
superset-frontend/src/explore/components/DatasourcePanel/DatasourceItems.tsx:
##########
@@ -105,6 +105,14 @@ export const DatasourceItems = ({
     ),
   );
 
+  useEffect(() => {
+    setCollapsedFolderIds(
+      new Set(
+        folders.filter(folder => folder.isCollapsed).map(folder => folder.id),
+      ),
+    );
+  }, [folders]);

Review Comment:
   Good catch, I agree. The initial implementation was resetting user-controlled collapse state on every folders update. I’ve updated the logic to initialize the collapsed state only once from folders, preserving subsequent user interactions while keeping the arrow state in sync.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(explore): update metric and column arrow state on datasource change [superset]

Posted by "YousufFFFF (via GitHub)" <gi...@apache.org>.
YousufFFFF commented on PR #37800:
URL: https://github.com/apache/superset/pull/37800#issuecomment-3873702395

   Hi @rusackas ! This PR is ready for review, and all CI checks are passing.
   I’d really appreciate your time and feedback when you get a chance.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(explore): update metric and column arrow state on datasource change [superset]

Posted by "YousufFFFF (via GitHub)" <gi...@apache.org>.
YousufFFFF commented on PR #37800:
URL: https://github.com/apache/superset/pull/37800#issuecomment-3888691650

   <img width="951" height="863" alt="image" src="https://github.com/user-attachments/assets/795d1ee8-c5b0-492a-afeb-b0ff8f051e16" />
   
   I noticed that this issue was reported on Superset version 5.0.0.
   I tested the behavior on the current master branch (latest development version) and was unable to reproduce it.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(explore): update metric and column arrow state on datasource change [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #37800:
URL: https://github.com/apache/superset/pull/37800#issuecomment-3874598112

   Via my handy ReviewBot3000:
   >  The pattern is a bit unusual. The component already initializes collapsedFolderIds from folders in useState. The useEffect catches cases where folders is empty/undefined on first render but populated later. This works, but a cleaner pattern might be to derive the initial state differently.
   
   I'll approve it, but maybe it needs a second pair of eyes from someone like @msyavuz 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(explore): update metric and column arrow state on datasource change [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #37800:
URL: https://github.com/apache/superset/pull/37800#issuecomment-3869791831

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(explore): update metric and column arrow state on datasource change [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #37800:
URL: https://github.com/apache/superset/pull/37800#issuecomment-3870189650

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(explore): update metric and column arrow state on datasource change [superset]

Posted by "YousufFFFF (via GitHub)" <gi...@apache.org>.
YousufFFFF commented on PR #37800:
URL: https://github.com/apache/superset/pull/37800#issuecomment-3870753310

   I’ve updated the implementation to address the concern about resetting user-controlled collapse state. The collapsed folder state is now initialized only once from `folders`, and subsequent updates preserve user interactions, avoiding unexpected uncollapse during search or other datasource updates.
   
   I’ve verified this locally:
   - Metric and column arrow state stays in sync
   - User expand/collapse actions are preserved across searches and datasource changes
   - Collapse state remains stable when switching datasets
   
   I’ve also added a short screen recording demonstrating the correct behavior after the change:
   
   https://github.com/user-attachments/assets/453cf604-3fc5-4e01-b36d-7a07f3608121
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(explore): update metric and column arrow state on datasource change [superset]

Posted by "YousufFFFF (via GitHub)" <gi...@apache.org>.
YousufFFFF commented on PR #37800:
URL: https://github.com/apache/superset/pull/37800#issuecomment-3888683690

   > Hey @YousufFFFF, i just checked out the attached issue and i can't reproduce it. If you can reproduce it can you also attach a before screen recording and detailed testing steps? For the implementation in this pr, you are setting collapsedFolderIds once right? That is already what the state initialization is doing. Am i missing something? Thanks for the help on this!
   
   Hi @msyavuz,
   Thanks for taking a look.
   I re-tested the behavior on the current master branch and was unable to reproduce the issue described in #37444. Collapsing the Metrics section no longer hides the Columns section, both sections behave independently as expected.
   It appears this may have already been resolved in a recent change. Since I cannot reproduce the issue on the latest codebase, I’ll go ahead and close this PR for now.
   Please feel free to reopen or comment if the issue is still reproducible on a specific version.
   
   Thanks again for the review.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org