EnxDev commented on PR #41636: URL: https://github.com/apache/superset/pull/41636#issuecomment-4901019658
## EnxDev's Review Agent — apache/superset#41636 · HEAD cd88821 **comment** — the parity upgrade is verified-complete and unusually well-tested; the only thing between this PR and green CI is two stale test selectors. Verification notes: I grepped the PR head for every class this PR renames (`ant-select-selector`, `ant-select-selection-search`, `ant-tabs-content-holder`, `ant-tabs-tabpane`, `ant-popover-inner`, `ant-collapse-content-box`, `ant-steps-item-tail`, `ant-spin-blur`, …) — outside comments and the contract test's negative assertions, only the two occurrences below survive, so the earlier "~15 components still on v5 selectors" thread is resolved at this HEAD. The remapped-meaning classes (`.ant-tabs-content`, which now matches the *panel*) are all correct-direction renames, including the behavioral one in `DashboardContainer.tsx:335` (`handleFocus`). `SemanticLayerModal`'s deleted inline `dropdownAlign` is covered by the wrapper's new `DROPDOWN_BUILTIN_PLACEMENTS` default (same points/offset/adjustX). Steps→`items` and Dropdown `overlay`→`menu` conversions preserve status conditionals, handlers, and style slots. ### 🟡 Should-fix - **`superset-frontend/packages/superset-ui-core/src/components/Select/Select.test.tsx:395`** and **`.../Select/AsyncSelect.test.tsx:415`** — both still query `.ant-select-selection-search-input` (gone in antd 6), so `trims whitespace from pasted comma-separated values` throws `Unable to fire a "paste" event` and keeps `sharded-jest-tests (1)` and `(6)` red on this HEAD. These two tests exist on master (`Select.test.tsx:381` / `AsyncSelect.test.tsx:401`) and were presumably picked up after the selector sweep → change both to `.ant-select-input` like the sibling paste tests. The PR body's "full Jest suite green" predates them. ### 🔵 Nits - Codecov patch coverage is 67.6% (12 lines). Flagged files include `Select.tsx` / `AsyncSelect.tsx` — likely the `Array.isArray(tokenSeparators) ? … : []` fallback (`Select.tsx:726`, `AsyncSelect.tsx:699`), which the narrowed `tokenSeparators?: string[]` in `types.ts` makes unreachable for type-checked callers. Could drop the guard or leave it as documented defense; either way not blocking. ### 🙌 Praise - `packages/superset-ui-core/src/components/antdDomContract.test.tsx` — pinning the `.ant-*` DOM contract with positive *and* negative assertions is exactly the safety net this class of upgrade needs; the next antd bump fails loudly instead of shipping silent CSS rot. The theme characterization snapshots and the `DROPDOWN_BUILTIN_PLACEMENTS` test pinning #36963 follow the same discipline. - `spec/helpers/jsDomWithFetchAPI.ts:65` — the MessageChannel polyfill is carefully engineered (bidirectional port pairing, macrotask delivery that can't resurrect the rc-overflow hang) and its rationale is documented in place. <!-- enxdev-review-agent:cd88821 --> _Reviewed by EnxDev's Review Agent — @EnxDev · HEAD cd88821._ -- 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]
