You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "SBIN2010 (via GitHub)" <gi...@apache.org> on 2025/03/18 20:17:39 UTC

[PR] Fix/upload data model [superset]

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

   The problem is that the Collapse component from the Ant Design library retains its state (such as an open panel) even after closing and reopening the modal window. This is because React does not destroy the component when the modal window is closed, but simply hides it. As a result, the Collapse state is preserved.
   
   To solve this problem, you need to reset the Collapse state every time you open the modal window, which is what this PR solves
   


-- 
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: upload data model Collapse state [superset]

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

   Thanks for the 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/upload data model [superset]

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

   /korbit-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

Re: [PR] Fix/upload data model [superset]

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


##########
superset-frontend/src/features/databases/UploadDataModel/index.tsx:
##########
@@ -539,6 +540,12 @@ const UploadDataModal: FunctionComponent<UploadDataModalProps> = ({
     }
   }, [delimiter]);
 
+  useEffect(() => {
+    if (show) {
+      setActiveKey('general');
+    }
+  }, [show]);

Review Comment:
   Good job on adding the documentation for the show effect. It would be more helpful if there was more detail about what the show effect is and why resetting activeKey is necessary when show changes. The name show effect is a little ambiguous without additional context, so it could be useful to include this in the comment.



-- 
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/upload data model [superset]

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


##########
superset-frontend/src/features/databases/UploadDataModel/index.tsx:
##########
@@ -539,6 +540,12 @@ const UploadDataModal: FunctionComponent<UploadDataModalProps> = ({
     }
   }, [delimiter]);
 
+  useEffect(() => {
+    if (show) {
+      setActiveKey('general');
+    }
+  }, [show]);

Review Comment:
   ### Missing documentation for show effect <sub>![category Documentation](https://img.shields.io/badge/Documentation-7c3aed)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The effect lacks documentation explaining why activeKey needs to be reset when show changes.
   
   ###### Why this matters
   Missing context makes it harder for other developers to understand the purpose of this side effect.
   
   ###### Suggested change ∙ *Feature Preview*
     // Reset active panel to 'general' when modal is shown
     useEffect(() => {
       if (show) {
         setActiveKey('general');
       }
     }, [show]);
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e5fb6bdf-89bf-4b0a-83d0-2642ab8277dd/upvote) [![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e5fb6bdf-89bf-4b0a-83d0-2642ab8277dd?what_not_true=true)  [![Not in Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e5fb6bdf-89bf-4b0a-83d0-2642ab8277dd?what_out_of_scope=true) [![Not in coding standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e5fb6bdf-89bf-4b0a-83d0-2642ab8277dd?what_not_in_standard=true) [![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e5fb6bdf-89bf-4b0a-83d0-2642ab8277dd)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:d9397743-5c51-4406-9bf5-929a325f8bb1 -->
   
   [](d9397743-5c51-4406-9bf5-929a325f8bb1)



-- 
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/upload data model [superset]

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

   If we can add a failing test for the "before" scenario, that'd be helpful.


-- 
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: upload data model Collapse state [superset]

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

   hi @rusackas i added the tests, is there anything else you need from me before this can be merged?


-- 
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/upload data model [superset]

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


##########
superset-frontend/src/features/databases/UploadDataModel/index.tsx:
##########
@@ -539,6 +540,12 @@ const UploadDataModal: FunctionComponent<UploadDataModalProps> = ({
     }
   }, [delimiter]);
 
+  useEffect(() => {
+    if (show) {
+      setActiveKey('general');
+    }
+  }, [show]);

Review Comment:
    documentation for show effect added



-- 
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: upload data model Collapse state [superset]

Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #32734:
URL: https://github.com/apache/superset/pull/32734


-- 
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: upload data model Collapse state [superset]

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

   > Если бы мы могли добавить провальный тест для сценария «до», это было бы полезно.
   
   I've added tests


-- 
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