rusackas commented on code in PR #36275:
URL: https://github.com/apache/superset/pull/36275#discussion_r2565979018
##########
superset-frontend/plugins/plugin-chart-word-cloud/src/plugin/controlPanel.tsx:
##########
@@ -63,25 +64,14 @@ const config: ControlPanelConfig = {
},
},
],
+ [<RotationControl name="rotation" key="rotation" renderTrigger />],
Review Comment:
This makes me wonder something about our general pattern here.
In this instance, we're all good, I think, since the props/values for this
component is always going to have the same options for the dropdown, and will
always be visible (since it's always relevant to the component).
But, let's say we're adding a React control for something more dynamic...
like the "Radius" control in the Pie chart. That one should only be
rendered/displayed if the "Donut" checkbox is checked. So, let's say you set
the radius control to a value, uncheck "Donut" and then check "Donut" again.
Would the value persist? If not, maybe we should consider adding a "hide" prop
that removes input visibility without un-rendering it.
Also, there's a likelihood that SOME controls will have dynamic values
passed in as props (based on other controls in the control panel). In those
cases more than this one, we'll probably want to memoize more. Memoization
doesn't seem necessary so much in this particular control, but I'm wondering if
we should just memoize things anyway, to establish a pattern for copypasting
our way through all control migrations. Thoughts?
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
