mistercrunch commented on code in PR #34178:
URL: https://github.com/apache/superset/pull/34178#discussion_r2249305763
##########
superset-frontend/src/features/reports/ReportModal/HeaderReportDropdown/index.tsx:
##########
@@ -85,63 +60,59 @@ const DropdownItemExtension = extensionsRegistry.get(
'report-modal.dropdown.item.icon',
);
-export enum CreationMethod {
- Charts = 'charts',
- Dashboards = 'dashboards',
-}
export interface HeaderReportProps {
dashboardId?: number;
chart?: ChartState;
- useTextMenu?: boolean;
- setShowReportSubMenu?: (show: boolean) => void;
- showReportSubMenu?: boolean;
- submenuTitle?: string;
showReportModal: () => void;
setCurrentReportDeleting: (report: AlertObject | null) => void;
}
-// Same instance to be used in useEffects
-const EMPTY_OBJECT = {};
-
-export default function HeaderReportDropDown({
+export const useHeaderReportMenuItems = ({
dashboardId,
chart,
- useTextMenu = false,
- setShowReportSubMenu,
- submenuTitle,
showReportModal,
setCurrentReportDeleting,
-}: HeaderReportProps) {
+}: HeaderReportProps): MenuItem | null => {
const dispatch = useDispatch();
- const report = useSelector<any, AlertObject>(state => {
- const resourceType = dashboardId
- ? CreationMethod.Dashboards
- : CreationMethod.Charts;
- return (
- reportSelector(state, resourceType, dashboardId || chart?.id) ||
- EMPTY_OBJECT
- );
+ const resourceId = dashboardId || chart?.id;
+ const resourceType = dashboardId
+ ? CreationMethod.Dashboards
+ : CreationMethod.Charts;
+
+ // Select the reports state and specific report with proper reactivity
+ const report = useSelector<any, AlertObject | null>(state => {
+ if (!resourceId) return null;
+ // Select directly from the reports state to ensure reactivity
+ const reportsState = state.reports || {};
+ const resourceTypeReports = reportsState[resourceType] || {};
+ const reportData = resourceTypeReports[resourceId];
+
+ // Debug logging to understand what's happening
Review Comment:
The thing that's really weird to me is that the console message appeared in
Explore ... Seems this component seems completely unrelated to explore (?) Yet
I see a lot of messages, meaning a lot of re-rendering is happening. I'm
surprised to see this. Worth an investigation as perf is probably generally
affected (?) Also seeing other weird stuff in the Network tab but that's
another story and not related to this PR ...
--
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]
