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 2021/02/12 12:47:13 UTC
[GitHub] [superset] kgabryje opened a new pull request #13094: fix: Fix switching viz type to and from Filter box
kgabryje opened a new pull request #13094:
URL: https://github.com/apache/superset/pull/13094
### SUMMARY
Switching viz type between a chart type and filter was broken due to a different type of data payload (array for charts and object for filter) and additional rerender caused by dispatching `RESET_FIELDS` after render caused by switching viz type. I moved the logic of resetting control fields to `SET_FIELD_VALUE` reducer, thus removing 1 rerender.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before: see https://github.com/apache/superset/issues/12624
After:
https://user-images.githubusercontent.com/15073128/107769388-32095280-6d38-11eb-9b1b-f083dba1b1d9.mov
### 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 -->
- [x] Has associated issue: fixes https://github.com/apache/superset/issues/12624
- [ ] Changes UI
- [ ] Requires DB Migration.
- [ ] Confirm DB Migration upgrade and downgrade tested.
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
CC: @junlincc @villebro @nikolagigic @rusackas
----------------------------------------------------------------
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] [superset] kgabryje commented on a change in pull request #13094: fix: Fix switching viz type to and from Filter box
Posted by GitBox <gi...@apache.org>.
kgabryje commented on a change in pull request #13094:
URL: https://github.com/apache/superset/pull/13094#discussion_r576270603
##########
File path: superset-frontend/src/explore/reducers/exploreReducer.js
##########
@@ -121,17 +121,35 @@ export default function exploreReducer(state = {}, action) {
});
const hasErrors = errors && errors.length > 0;
+ const controls =
+ action.controlName === 'viz_type' &&
+ action.value !== state.controls.viz_type.value
+ ? {
+ ...getControlsState(
+ state,
+ getFormDataFromControls({
+ ...state.controls,
+ viz_type: control,
+ }),
+ ),
+ [action.controlName]: {
+ ...control,
+ validationErrors: errors,
+ },
+ }
+ : {
+ ...state.controls,
+ [action.controlName]: {
+ ...control,
+ validationErrors: errors,
+ },
+ };
Review comment:
Good point! Done
----------------------------------------------------------------
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] [superset] junlincc merged pull request #13094: fix: Fix switching viz type to and from Filter box
Posted by GitBox <gi...@apache.org>.
junlincc merged pull request #13094:
URL: https://github.com/apache/superset/pull/13094
----------------------------------------------------------------
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] [superset] ktmud commented on a change in pull request #13094: fix: Fix switching viz type to and from Filter box
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #13094:
URL: https://github.com/apache/superset/pull/13094#discussion_r576099738
##########
File path: superset-frontend/src/explore/reducers/exploreReducer.js
##########
@@ -121,17 +121,35 @@ export default function exploreReducer(state = {}, action) {
});
const hasErrors = errors && errors.length > 0;
+ const controls =
+ action.controlName === 'viz_type' &&
+ action.value !== state.controls.viz_type.value
+ ? {
+ ...getControlsState(
+ state,
+ getFormDataFromControls({
+ ...state.controls,
+ viz_type: control,
+ }),
+ ),
+ [action.controlName]: {
+ ...control,
+ validationErrors: errors,
+ },
+ }
+ : {
+ ...state.controls,
+ [action.controlName]: {
+ ...control,
+ validationErrors: errors,
+ },
+ };
Review comment:
These lines are a little difficult to grok, creating a new variable might make it easier to read:
```js
const currentControlsState =
action.controlName === 'viz_type' &&
action.value !== state.controls.viz_type.value
? // rebuild the full control state if switching viz type
getControlsState(
state,
getFormDataFromControls({
...state.controls,
viz_type: control,
}),
)
: state.controls;
return {
...state,
form_data: new_form_data,
triggerRender: control.renderTrigger && !hasErrors,
controls: {
...currentControlsState,
[action.controlName]: {
...control,
validationErrors: errors,
},
},
};
```
----------------------------------------------------------------
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] [superset] junlincc commented on pull request #13094: fix: Fix switching viz type to and from Filter box
Posted by GitBox <gi...@apache.org>.
junlincc commented on pull request #13094:
URL: https://github.com/apache/superset/pull/13094#issuecomment-778901454
approving as product sign-off. Thanks for the fix! 🙂 @villebro ready for ya.
https://user-images.githubusercontent.com/67837651/107899533-637a4d80-6ef3-11eb-8ccf-fb547b5d1922.mov
----------------------------------------------------------------
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