sadpandajoe commented on PR #43531: URL: https://github.com/apache/superset/pull/43531#issuecomment-5516298656
Pushed `ff81c8cd13` addressing the open review feedback: **CI (@aminghadersohi's shard-3 finding, confirmed):** the only failure was `SqlEditor.test.tsx › disables the save dataset button...` with `SyntaxError: 'button,,,<hash>-more...' is not a valid selector` — an emotion class-hash collision in a file this PR never touches. It passes standalone locally (`npx jest src/SqlLab/components/SqlEditor/SqlEditor.test.tsx`). Re-ran the job; the new push now re-triggers the full suite anyway. **`theme.sizeXS` vs `theme.fontSizeXS` (@aminghadersohi's round-2 question):** confirmed intentional, not an oversight — added a comment at both call sites explaining why. `fontSizeXS` is a Superset custom token seeded only via `THEME_DEFAULT` in `config.py` (`"fontSizeXS": "8"`), so it's `undefined` in any context that builds a `Theme` without that seed (Storybook, Jest, docs preview) — `Theme.fromConfig()` with no args, as used by `packages/superset-core/src/theme/index.tsx`'s module-level singleton, never sets it. `sizeXS` is an antd-native token, always computed by `getDesignToken()` regardless of config, and happens to default to `8` as well — same rendered result in the real app, but robust in the environments where `fontSizeXS` isn't seeded. Using `fontSizeXS` there would have reintroduced the original oversized-icon symptom in exactly those contexts. **@rebenitez1802's CHANGES_REQUESTED:** that review was against `6b10958e`; the `Menu.tsx` left-nav gap it flagged was closed in `fd4a737aaa` / `065d11f676`, and @aminghadersohi's follow-up at `98cabfdd` confirmed all five call sites now render the same icon with no site left behind. Flagging here so the stale request-changes can be re-evaluated against current HEAD — not resolving it myself since it's not my review to dismiss. -- 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]
