rusackas commented on PR #38535:
URL: https://github.com/apache/superset/pull/38535#issuecomment-4883687096

   @yousoph checking in on this one... the feature itself is still worth 
landing, but there's a pile-up between the branch and where master has moved, 
so I can't approve as-is.
   
   GitHub shows conflicts on 5 of the 9 files, and the last CI run (April) has 
two real jest failures in suites this PR touches (AlertReportModal timeout, 
FileHandler heap OOM).
   
   The rebase also isn't mechanical, unfortunately. Two things landed on master 
since: granular export controls (#38581) now permission-gate image export in 
the Explore menu, so every new PNG/PDF item needs the same `disabled: 
imageExportDisabled` treatment or it bypasses the gate... and #38781 added a 
second AG-Grid-specific export path in `downloadAsImage.tsx`, so the 
PNG/transparent logic needs to cover that call site too or table charts 
silently keep exporting jpeg.
   
   Joe's earlier asks are still open as well: reuse 
`getExportScreenshotMenuItems` instead of the inline duplicate in the Explore 
menu, tests for that helper, and a Solid-background test. The two bot threads I 
just confirmed above (the `.finally()` restore and the z-index stacking-context 
risk) are real too.
   
   Happy to re-review after the rebase and those items... holler if help here 
is needed.


-- 
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]

Reply via email to