yousoph commented on PR #38535: URL: https://github.com/apache/superset/pull/38535#issuecomment-4906952504
Pushed a commit addressing the outstanding review feedback (rebased on latest `master`, so the branch is mergeable again): - **Dropdown overlay left permanently hidden on capture failure** (@codeant-ai-for-open-source, @rusackas) **+ z-index stacking context** (@Copilot, @alexandrusoare) **+ "is this needed?"** (@msyavuz): removed the custom `dropdownOverlayContainerRef` popup container entirely and reverted to master's `.ant-dropdown:not(.ant-dropdown-hidden)` visibility hack. All three handlers now restore visibility in a `.finally()` so a failed screenshot/PDF can't leave the menu invisible. - **Duplicated PNG/PDF export code + hard-coded selector** (@sadpandajoe, @Copilot, @rusackas): parameterized `getExportScreenshotMenuItems` (menu keys) and replaced the second menu's inline PNG submenu + PDF item with a spread of the helper; both call sites now use `CHART_EXPORT_SELECTOR` (including the jpeg handlers). - **Missing log-action registration** (@sadpandajoe): added `LOG_ACTIONS_CHART_DOWNLOAD_AS_PNG` / `_AS_PDF` to `LOG_EVENT_TYPE_USER`. - **Missing tests** (@sadpandajoe): exported and unit-tested `getExportScreenshotMenuItems`, and added the solid-background counterpart test in `SliceHeaderControls.test.tsx`. PNG labels already read `Export screenshot (PNG)`. Per @rusackas, the `AlertReportModal.test.tsx` `queryAllByText` tightening suggestions were left as pre-existing / out of scope. -- 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]
