OrhanBC commented on code in PR #36275:
URL: https://github.com/apache/superset/pull/36275#discussion_r2570111227
##########
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:
I’m leaning towards not using a blanket memoization approach. Looking at the
codebase, React.memo is only used in 2 controls (FastVizSwitcher,
ColumnConfigItem), both with clear performance needs. The implicit pattern
seems to be: only memoize when there's a demonstrated need.
For controls with dynamic props and computed options, we should use
useMemo/useCallback for those computed values, but not necessarily wrap the
whole component in React.memo unless we see performance issues.
Let's keep this simple and address performance on a case-by-case basis as we
encounter more complex controls during migration.
--
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]
