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

   Update: the capture tool now grabs every distinct chart per viz type instead 
of one representative, so gallery slots with multiple examples show real 
variety. Ran it for real against a live instance this time, which surfaced a 
couple of things worth calling out.
   
   First, digging into coverage turned up two of the existing "dark" thumbnails 
that were flat-out faked: `ag-grid-table`'s and `cartodiagram`'s dark variants 
were a naive full-image color inversion of the light one, not an actual 
dark-theme render (checked by pixel-diffing every light/dark pair in the repo, 
these two matched a naive invert at 85-99% while every real capture matched 
under 2%). Renamed every pre-existing thumbnail with a `custom_thumb_` prefix 
so it's honest about being hand-made art rather than a live capture, and 
flipped each plugin back to the real filename wherever a genuine capture 
succeeded.
   
   Second, running this live against real dashboards found two actual bugs, 
unrelated to any of this thumbnail work but only surfaced by it: [Bubble's 
transformProps had `xAxis.interval` set at the wrong config 
level](https://github.com/apache/superset/commit/49a4086715), crashing every 
bubble_v2 chart at its default control value, and the rose viz type's thumbnail 
path pointed at a package consolidated away a while back. Both fixed, with a 
regression test for the Bubble one since there wasn't any before.
   
   On the inventory you asked about, four viz types still have zero chart on 
any example dashboard:
   - 
[`pop_kpi`](https://github.com/apache/superset/blob/fb5029d06d/superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberPeriodOverPeriod/index.ts)
 and 
[`ag-grid-table`](https://github.com/apache/superset/blob/fb5029d06d/superset-frontend/plugins/plugin-chart-ag-grid-table/src/index.ts)
 are gated behind `ChartPluginsExperimental`/`AgGridTableEnabled` and 
unregistered by default, so an example chart would just show every regular user 
a broken card.
   - 
[`deck_multi`](https://github.com/apache/superset/blob/fb5029d06d/superset-frontend/plugins/preset-chart-deckgl/src/Multi/index.ts)
 stores its sub-layers as raw DB-assigned chart ids in `deck_slices`, and 
there's no UUID-based path for the YAML importer to remap those on a fresh 
load. Needs an importer/exporter feature, not something a fixture alone can fix.
   - 
[`cartodiagram`](https://github.com/apache/superset/blob/fb5029d06d/superset-frontend/plugins/plugin-chart-cartodiagram/src/plugin/index.ts)
 embeds a serialized sub-chart config, which needs careful hand-crafting to get 
right.
   
   `deck_geojson` was on that list too but turned out tractable: derived a 
`geojson` column from the existing polygon data in the deck.gl demo dataset and 
added a real example chart for it.


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