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


   ### SUMMARY
   Currently enabling cascading in native filters requires instantly applying 
filters, causing all charts in scope to rerender and retrigger queries. This PR 
changes child filters to apply the currently selected data mask of the parent 
filter instead of the currently applied one, removing the need to instantly 
apply filters.
   
   While making the changes, I considered moving `dataMaskSelected` into Redux 
instead of adding it as a prop on the chain of components. However, I noticed 
that many of the intermediate components between the filter tab and 
`FilterValue` were already using `dataMaskSelected`, which made the benefits of 
moving it to Redux less appealing. In addition, adding it to Redux would have 
required either
   - introducing a new `dataMaskSelected` property to complement the `dataMask` 
property in the store (which contains the applied state)
   
   or 
   - refactoring `dataMask` so that it would be split into `dataMask.applied` 
and `dataMask.selected`
   
   Since both of these would have required fairly significant non-functional 
changes, it feels better to perform this refactoring later when the full scope 
of refactoring needs is clear (e.g. will some of this state be moved into 
Context or be kept in Redux).
   
   ### BFEORE
   Currently each time a parent filter is changed, it triggers the filter 
automatically:
   
https://user-images.githubusercontent.com/33317356/120623645-962b6000-c468-11eb-86b5-db95319691f4.mp4
   
   ### AFTER
   Now the parent filter only retriggers the child filters:
   
https://user-images.githubusercontent.com/33317356/120623687-9e839b00-c468-11eb-8e1e-ea6a85443d2a.mp4
   
   ### TESTING INSTRUCTIONS
   1. Create a dashboard with cascading filters
   2. Make sure the parent filter is *not* set to "Apply changes instantly"
   3. Change the selection in the parent filter
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [ ] Has associated issue:
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in 
[SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [ ] 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:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to