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 2019/05/21 18:19:27 UTC

[GitHub] [incubator-superset] felixcodes opened a new pull request #7569: [WIP] Separate vis-specific controls from centralized controls

felixcodes opened a new pull request #7569: [WIP] Separate vis-specific controls from centralized controls
URL: https://github.com/apache/incubator-superset/pull/7569
 
 
   ### CATEGORY
   - [x] Enhancement (new features, refinement)
   
   ### SUMMARY
   This PR is a proof of concept that allows each visualization to define its own controls, instead of storing all controls in the centralized `controls.jsx` file. It still offers support for shared controls defined in `controls.jsx`. Moving forward, we would want to create a registry for controls that are defined at the visualization level. This would allow us to move towards a modularized world where new charts can be installed and maintained as plugins, reducing bloat in our codebase. 
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   ![good_behavior](https://user-images.githubusercontent.com/17525561/58120275-beb1cb80-7bb9-11e9-85bc-3a2706e27cae.gif)
   
   ### TEST PLAN
   Verified that the `rose` chart modified in this PR is working correctly*
   
   *when the `vizType` bug described below does not interfere with functionality
   
   ### ADDITIONAL INFORMATION
   - [x] Has associated issue:
   This PR uncovered an issue with the `vizType` form data property not being updated when you switch from one visualization to another (ex. Line to Rose, Rose to Pie, etc.). This has likely gone unnoticed because chart controls used to rely on the `controls.jsx` index -- not `vizType`. This new PR requires the correct `vizType` to be passed in so the correct control panel config can be identified and looked up against.
   
   The UI implications of this bug are shown in the GIF before. Note that desired behavior is for the `Use Area Proportions` checkbox control is an automatic re-render, as shown in the before/after animated gif section above.
   
   ![bad_behavior](https://user-images.githubusercontent.com/17525561/58120084-55ca5380-7bb9-11e9-9a60-46aff28e6353.gif)
   
   ![image](https://user-images.githubusercontent.com/17525561/58119855-d5a3ee00-7bb8-11e9-8695-55b39b370406.png)
   
   ### REVIEWERS
   @kristw @williaster 

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


With regards,
Apache Git Services

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