amaannawab923 commented on PR #43820: URL: https://github.com/apache/superset/pull/43820#issuecomment-5540308281
did a round of manual testing on this branch with a small dataset and hit a few cases worth a look. all of these are on the table chart, so they go through the shared `getColorFormatters` path rather than anything plugin specific. **1. a column of all negative values gets no colour at all** same formatting rule on both columns, gradient with percent bounds, 0 to 100 percent of max. `neg_col` is -10 to -40, `pos_col` is 10 to 40. <img width="2454" height="248" alt="cf-01-negative-column-no-color" src="https://github.com/user-attachments/assets/46cd25c2-f5d1-4c38-b3b7-42d3f826081e"; /> `pos_col` gets the full gradient, `neg_col` gets nothing. `resolvePercentBound` returns `(bound / 100) * denominatorValue` with the denominator wrapped in `Math.abs`, so for any non negative bound percentage the resolved bound is always >= 0 and no bound can ever land below zero. every value in an all negative column then falls under the lower bound and comes out at zero alpha. dropping the `Math.abs`, or resolving the percent against the actual min/max range of the column, would probably cover it. **2. percent of sum saturates when the signed sum lands near zero** profit by region, same gradient but with `percentDenominator: sum`: <img width="2454" height="248" alt="cf-02-percent-of-sum-saturates" src="https://github.com/user-attachments/assets/8202f7a1-dbd5-4145-85a1-a959836bdf69"; /> the signed sum here is 1, so 100 percent resolves to a bound of 1 and both positive rows saturate to the same red. the 25 percent gap between north and east is invisible, and the two negative rows read as blank cells rather than as bad values. there is already a `denominatorValue === 0` guard so the case looks anticipated, it just only catches the exact zero point. summing absolute values for the denominator would handle it, and would also make percent of sum read as share of total, which is probably closer to what someone picking that option expects. **3. center value sitting on the min or max drops the diverging config with no feedback** the description calls this fallback out as intended, so this one is more of a ux note than a bug. three colour diverging, bounds 10 to 40, only `centerValue` changes between the two runs. center 25: <img width="2454" height="248" alt="cf-03a-center-25-diverging-works" src="https://github.com/user-attachments/assets/80e0aad5-32ea-46ae-8768-1ee35f529058"; /> center 40: <img width="2454" height="248" alt="cf-03b-center-40-diverging-dropped" src="https://github.com/user-attachments/assets/bbeb3772-9756-47ca-9038-6eab7c891655"; /> the three colours are dropped and it falls back to the single hue `colorScheme`. `isValidDivergingConfig` uses strict `>` and `<` against cutoff and extreme, so a center sitting exactly on either end fails validation. the fallback itself is reasonable, but from the panel the rule still looks saved and correct, so there is nothing to tell you it was ignored. surfacing it in the control would save some head scratching. <details><summary>repro data</summary> ```sql -- cases 1 and 3 SELECT 'A' AS label, -10 AS neg_col, 10 AS pos_col UNION ALL SELECT 'B', -20, 20 UNION ALL SELECT 'C', -30, 30 UNION ALL SELECT 'D', -40, 40; -- case 2 SELECT 'North' AS region, 500 AS profit UNION ALL SELECT 'South', -499 UNION ALL SELECT 'East', 400 UNION ALL SELECT 'West', -400; ``` </details> -- 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]
