RehanIslam09 commented on issue #40428:
URL: https://github.com/apache/superset/issues/40428#issuecomment-4958963723
Hi,
I've been investigating this issue locally and wanted to share my findings
to help move this toward a resolution.
I traced the root cause of the visual overlap directly to the
`processCloneForVisibility()` function located in
`superset-frontend/src/utils/downloadAsImage.tsx`.
### The Core Conflict
Through isolated testing, I discovered that the overlap issue disappears
entirely if I remove the broad `[style*="overflow"]` selector from
`scrollableSelectors`.
However, simply removing or omitting this selector immediately reintroduces
the regression that was originally resolved in PR #34233 ("ensure full table or
handlebar chart is captured in image export"). When it is removed, table charts
revert to exporting only the visible viewport rows rather than the entire
dataset.
### Technical Approaches Evaluated
To find a middle ground, I attempted several targeted adjustments to narrow
the scope of the selector without breaking the fix from #34233. Unfortunately,
none of these approaches successfully preserved both behaviors simultaneously:
* **Scoped Class Targeting:** Restricting the selector explicitly to
`.superset-chart-table` or targeting only `role="presentation"` containers
caused the table export to revert to copying only visible rows.
* **Dynamic Style Evaluation:** Swapping the string-based inline style
match for an explicit `getComputedStyle()` check did not resolve the overlap
behavior.
* **Lifecycle and Timing Adjustments:** Introducing a slight delay to
allow for additional layout reflows prior to executing `domToImage.toJpeg()`
yielded no change.
* **Regression History Analysis:** I audited the implementation against
the original #34233 commit and subsequent PRs (such as #38927) to check for
introduced side effects, but could not isolate a specific divergence.
### Key Observations
1. **Export Path Specificity:** The overlap reproduces exclusively on the
standard clone-based export pipeline (`createEnhancedClone()` /
`processCloneForVisibility()`). The AG Grid export pipeline remains unaffected.
2. **Container Mechanics:** While the actual scroll container for the table
appears to be a `role="presentation"` element with `overflow: auto`, the
full-table export relies on expanding multiple parent or ancestor elements
matching the broad overflow criteria, rather than just the immediate scroll
container itself.
Because the full-table export and the visual overlap appear to be tightly
coupled within the current architecture, resolving this cleanly will likely
require a more nuanced approach than just modifying the selector query.
### Request for Context
To help guide a proper fix, could anyone provide additional context on why
PR #34233 intentionally targeted every element matching `[style*="overflow"]`?
I am curious if there was a specific deeply-nested DOM architecture or a
particular browser-specific limitation that mandated such a broad dragnet at
the time. Understanding that underlying constraint will be incredibly helpful
in figuring out how to fix #40428 without regressing #34233.
Thanks in advance for any insights!
--
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]
