sadpandajoe opened a new pull request, #44430:
URL: https://github.com/apache/superset/pull/44430

   ### SUMMARY
   
   A table chart that mounts inside a `display: none` subtree — an inactive 
dashboard tab — measures its header against zero-sized boxes, so `StickyWrap`'s 
layout effect bails out at its `if (!theadHeight) return;` guard, computes no 
sticky layout, and renders only its `visibility: hidden` sizer. The chart is 
blank even though its query response arrived successfully.
   
   Nothing brings it back. The measurement effect's dependencies are 
`maxWidth`, `maxHeight`, `setStickyState` and the scrollbar size; the width and 
height are static grid-layout math from `ChartHolder`, not measurements, so 
none of them change when the tab becomes visible. The measurement never runs 
again and the chart stays blank until it is force-refreshed (a remount, this 
time while visible) or the window is resized (a real width change) — exactly 
the two workarounds in #44415.
   
   This is why the issue reproduces only when the filter is applied *from 
another tab*: applying it on the chart's own tab does the one and only 
measurement while the chart has a box. It is also why only some charts blank — 
ECharts-based charts pass `width`/`height` explicitly into `init()` 
(`plugins/plugin-chart-echarts/src/components/Echart.tsx`, with a comment 
saying it exists to avoid depending on DOM size) and are immune.
   
   **No introducing commit exists in this repository.** `git log -S` shows both 
the `if (!theadHeight) return;` guard and the `[maxWidth, maxHeight, 
setStickyState, scrollBarSize]` dependency list arriving with the monorepo 
import, 3c41ff68a4 (#17552), so they came in already-formed from the external 
`superset-ui` repo. For history: #10432 (2020) fixed the same class of bug in 
the then-separate `@superset-ui/plugin-chart-table` via a version bump, and 
#22009 (2022) fixed the dashboard-side resize trigger in the old class-based 
`Chart.jsx`; that dashboard-side guard has no equivalent in today's hooks-based 
`Chart.tsx`, but it would not help here either, since the chart's 
`width`/`height` props genuinely do not change on a tab switch.
   
   The fix observes the sticky wrapper with a `ResizeObserver` and measures 
again once it gains a box. The observer is attached only while no sticky layout 
has been computed and disconnects as soon as one exists, so charts that 
measured normally are untouched; `measure` re-reads the DOM itself, so a 
still-boxless notification is a no-op. A stable wrapper ref is observed rather 
than the existing `theadRef`, because that ref is only attached while the sizer 
is rendered and its identity changes across renders without signalling the 
effect.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   No screenshots: no running Superset instance was available in the 
environment where this was developed, so a genuine before/after of the blank 
chart could not be captured. The failure and the fix are demonstrated instead 
by the added regression test, which fails on `master` and passes with this 
change.
   
   ### TESTING INSTRUCTIONS
   
   1. Create a dashboard with two tabs. Put a Table chart on tab 1; leave tab 2 
empty (or put an unrelated chart on it).
   2. Add a native filter (e.g. a Year value filter) scoped to the Table chart 
on tab 1.
   3. Load the dashboard so tab 1 renders and the table paints, then switch to 
tab 2.
   4. From tab 2, pick a filter value and click **Apply filters**.
   5. Switch back to tab 1. The table should render the filtered rows 
immediately. On `master` it is blank, and only a per-chart **Force refresh** or 
a browser-window resize brings it back.
   6. Sanity check that nothing regressed for the normal path: apply a filter 
while already on tab 1, resize the window, and page through the table — the 
sticky header and column widths should behave as before.
   
   ### ADDITIONAL INFORMATION
   - [x] Has associated issue: Fixes #44415
   - [ ] Required feature flags:
   - [x] Changes UI
   - [ ] Includes DB Migration (follow approval process in 
[SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)


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