You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2020/07/30 10:41:47 UTC

[GitHub] [incubator-superset] villebro opened a new pull request #10482: fix: enforce mandatory chart name on save and edit

villebro opened a new pull request #10482:
URL: https://github.com/apache/incubator-superset/pull/10482


   ### SUMMARY
   Currently it is possible to save charts without names, causing confusion in the React CRUD list view. This PR changes the following:
   - Enforce the required slice name field when editing a chart on the React CRUD view.
   - Migrate the `ControlLabel` to `FormLabel` on `SaveModal.tsx` and add `required props to chart and dashboard and enforce required chart name on both "Save" and "Save & Go To Dashboard".
   
   ### BEFORE
   ![chart-react-before](https://user-images.githubusercontent.com/33317356/88912599-2b1d6580-d268-11ea-8933-a8d8ec6aaeea.gif)
   ![Jul-30-2020 13-34-30](https://user-images.githubusercontent.com/33317356/88913374-72582600-d269-11ea-9329-2c31d5a2d5e4.gif)
   
   ### AFTER
   ![Jul-30-2020 13-29-02](https://user-images.githubusercontent.com/33317356/88912923-b4cd3300-d268-11ea-82d7-f23610554be2.gif)
   ![Jul-30-2020 13-33-21](https://user-images.githubusercontent.com/33317356/88913393-797f3400-d269-11ea-9159-01b751a117e9.gif)
   
   ### TEST PLAN
   <!--- What steps should be taken to verify the changes -->
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [ ] Has associated issue:
   - [ ] Changes UI
   - [ ] Requires DB Migration.
   - [ ] Confirm DB Migration upgrade and downgrade tested.
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   


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

[GitHub] [incubator-superset] villebro merged pull request #10482: fix: enforce mandatory chart name on save and edit

Posted by GitBox <gi...@apache.org>.
villebro merged pull request #10482:
URL: https://github.com/apache/incubator-superset/pull/10482


   


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

[GitHub] [incubator-superset] Steejay commented on pull request #10482: fix: enforce mandatory chart name on save and edit

Posted by GitBox <gi...@apache.org>.
Steejay commented on pull request #10482:
URL: https://github.com/apache/incubator-superset/pull/10482#issuecomment-666398063


   @villebro pattern looks good.
   
   can we switch the button order of the Edit Chart Properties modal? Move **Save** to the right and **Cancel** to the left. sim to the Save Chart arrangement. 
   
   can we also change the color of the active state for the **Save** button to #20A7C9? right now both active and disabled state are grey. 


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

[GitHub] [incubator-superset] Steejay commented on pull request #10482: fix: enforce mandatory chart name on save and edit

Posted by GitBox <gi...@apache.org>.
Steejay commented on pull request #10482:
URL: https://github.com/apache/incubator-superset/pull/10482#issuecomment-666410583


   @villebro great! thanks Ville.


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

[GitHub] [incubator-superset] villebro commented on pull request #10482: fix: enforce mandatory chart name on save and edit

Posted by GitBox <gi...@apache.org>.
villebro commented on pull request #10482:
URL: https://github.com/apache/incubator-superset/pull/10482#issuecomment-666408412


   @Steejay got it; I switched the order of the buttons. I think the colors are off in the animation, here's what they actually look like:
   ![image](https://user-images.githubusercontent.com/33317356/88936370-814fd000-d28b-11ea-8ad2-13a4a48a91ba.png)
   


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