krishn1301 commented on PR #42854: URL: https://github.com/apache/superset/pull/42854#issuecomment-5278502389
@sadpandajoe added a before/after to the description. @rusackas the `ValueFormatter` typing is fixed in a5b6e03 — `tsc --noEmit` passes and CI is green now. On the before/after: I don't have a Superset instance running locally, so instead of mocking something up by hand I captured the real label strings from this plugin's `transformSeries` formatter — once with the fix and once with the `numericValue !== 0` guard removed — and rendered them with the same ECharts config the plugin builds. The overlap you can see in the "before" panel is exactly what the formatter returns: `'0'` at the same `label.position: 'top'` anchor as the adjacent segment's label. Say the word if you'd rather have screenshots from a live instance and I'll find a way to stand one up. One thing worth a human opinion, since two review bots flagged it: they point out that a non-stacked series whose legend entry is deselected skips the `!stack && isSelectedLegend` branch and reaches the new zero check, so its `0` labels would now be suppressed too. That's true of the code path, but the series is hidden when its legend entry is deselected, so no label is drawn either way and I don't think the change is observable. I left it as is rather than adding a `stack &&` guard for a case that can't render. Glad to add it if you'd prefer the condition to be explicit about intent. -- 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]
