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

   ### SUMMARY
   Matrixify grid cells did not participate in the existing 
`DASHBOARD_VIRTUALIZATION` mechanism that other dashboard content (`Row.tsx`) 
already uses to avoid firing every chart's query at once. As a result, a large 
Matrixify matrix (e.g. 10x10 = 100 cells) fires 100 chart queries in the same 
tick regardless of how many cells are actually visible, even with the flag 
enabled.
   
   This fixes that gap: each Matrixify grid cell now defers mounting its 
`StatefulChart` (and therefore its data query) until the cell scrolls into 
view, using an `IntersectionObserver`, mirroring the lazy mount/unmount pattern 
`Row.tsx` already uses for the same flag - applied at the individual-cell level 
instead of the row level.
   
   A one-way latch keeps a cell mounted once it has entered the viewport, so 
scrolling back and forth doesn't cause refetch churn.
   
   Two existing export paths are respected so captures still include the full 
matrix regardless of scroll position:
   - **Server-side** screenshot/report/thumbnail workers set 
`navigator.webdriver`; cells render immediately in that case.
   - **Client-side** "Download as Image/PDF" dispatches `FORCE_IN_VIEW_EVENT` 
(`src/utils/downloadUtils.ts`), optionally scoped to a batch of dashboard row 
ids for large dashboards (`forceLoadAllCharts`'s row-batching). 
`MatrixifyGridCell` mirrors the same scoping `Row.tsx` applies (via the nearest 
ancestor `[data-row-id]`), so it only force-mounts when its own row's batch 
fires (or the dispatch is unscoped). Without this, a Matrixify chart in an 
already-mounted row would dump every deferred cell's query into whichever batch 
happens to fire first, defeating the purpose of `downloadUtils.ts`'s row 
batching.
   
   The event-name constants are duplicated (not imported) from 
`src/dashboard/constants.ts` into a new 
`packages/superset-ui-core/.../Matrixify/virtualizationEvents.ts`, since 
`packages/superset-ui-core` cannot depend on app code under `src/dashboard`.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   N/A - this changes query timing/count, not visual appearance. Off-screen 
cells render a same-sized empty placeholder instead of the chart until they 
scroll into view; there is no visible layout change.
   
   ### TESTING INSTRUCTIONS
   1. Enable the `DASHBOARD_VIRTUALIZATION` feature flag.
   2. Create a dashboard with a Matrixify chart whose grid has more 
rows/columns than fit in the viewport (e.g. a 5x5 or larger grid).
   3. Open the browser Network tab, load the dashboard, and confirm only the 
cells currently in view (plus a ~200px buffer) fire chart-data requests; 
scrolling down triggers additional requests only for newly-revealed cells.
   4. Scroll a cell out of view and back - confirm it does not re-fire its 
query (latch).
   5. With the flag still on, use the dashboard's "Download as Image" or 
"Download as PDF" action on a dashboard containing this Matrixify chart, and 
confirm the exported file includes every cell's chart, not blank placeholders 
for the ones that were off-screen.
   6. Disable the feature flag and confirm all cells render immediately as 
before (no behavior change).
   
   Unit tests added in `MatrixifyGridCell.test.tsx` cover: eager rendering with 
the flag off, deferred mount + IntersectionObserver-triggered mount, the mount 
latch, headless-capture eager rendering, unscoped and row-scoped 
`FORCE_IN_VIEW_EVENT` handling (including a cell mounting for the first time 
after its row's batch has already fired), and re-arming after 
`RESTORE_VIRTUALIZATION_EVENT`.
   
   ### ADDITIONAL INFORMATION
   - [ ] Has associated issue:
   - [x] Required feature flags: `DASHBOARD_VIRTUALIZATION` (existing flag; 
this fixes Matrixify's per-cell rendering to respect it, it does not add a new 
flag)
   - [ ] Changes UI
   - [ ] Includes DB Migration
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API


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