superset-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [incubator-superset] etr2460 commented on a change in pull request #9714: fix bug where error at import dashboard fails to show toast in "welcome" app
Date Sat, 02 May 2020 03:01:14 GMT

etr2460 commented on a change in pull request #9714:
URL: https://github.com/apache/incubator-superset/pull/9714#discussion_r418849441



##########
File path: superset-frontend/src/welcome/App.jsx
##########
@@ -56,7 +56,7 @@ const store = createStore(
   {},
   compose(applyMiddleware(thunk), initEnhancer(false)),
 );
-
+console.log('store', store)

Review comment:
       remove console log

##########
File path: superset-frontend/src/welcome/App.jsx
##########
@@ -68,7 +68,7 @@ const App = () => (
               <Welcome user={user} />
             </Route>
             <Route path="/dashboard/list/">
-              <DashboardList user={user} />
+              <DashboardList user={user} common={common} />

Review comment:
       maybe we only pass the flash_messages here if that's all that's needed?

##########
File path: superset-frontend/src/views/dashboardList/DashboardList.tsx
##########
@@ -82,6 +85,11 @@ class DashboardList extends React.PureComponent<Props, State> {
   };
 
   componentDidMount() {
+    let messages = this.props.common.flash_messages;
+    if(messages.length > 0){
+      let text = messages[0][1]
+      this.props.addDangerToast(text)

Review comment:
       i believe the flash_messages object should have a level associated with it, so we can
show the proper toash (info/warning/danger)

##########
File path: superset-frontend/src/views/dashboardList/DashboardList.tsx
##########
@@ -82,6 +85,11 @@ class DashboardList extends React.PureComponent<Props, State> {
   };
 
   componentDidMount() {
+    let messages = this.props.common.flash_messages;
+    if(messages.length > 0){

Review comment:
       what happens if there are multiple messages? should we show multiple toasts?

##########
File path: superset-frontend/src/views/dashboardList/DashboardList.tsx
##########
@@ -39,6 +39,9 @@ const PAGE_SIZE = 25;
 interface Props {
   addDangerToast: (msg: string) => void;
   addSuccessToast: (msg: string) => void;
+  common: {
+    flash_messages: any[];

Review comment:
       can we add more explicit typing here?




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

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


Mime
View raw message