codeant-ai-for-open-source[bot] commented on code in PR #40970:
URL: https://github.com/apache/superset/pull/40970#discussion_r3395415488
##########
superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx:
##########
@@ -554,6 +572,14 @@ export default function PluginFilterSelect(props:
PluginFilterSelectProps) {
return (
<FilterPluginStyle height={height} width={width}>
+ {inverseSelection && (
+ <style>{`
+ .inverse-select-dropdown .ant-select-item {
+ margin-right: 14px !important;
+ border-radius: 4px !important;
+ }
+ `}</style>
+ )}
Review Comment:
**Suggestion:** Rendering a raw `<style>` block inside the component creates
duplicate global style nodes for every inverse filter instance and on remounts,
which is an avoidable DOM/perf cost and makes styling harder to control. Move
these rules into existing styled-component CSS (scoped by class/prop) so styles
are emitted once and remain component-scoped. [performance]
<details>
<summary><b>Severity Level:</b> Minor ๐งน</summary>
```mdx
- โ ๏ธ Extra style tags rendered for each inverse-selection filter instance.
- โ ๏ธ Suggestion is a micro-optimization; existing styling works correctly.
```
</details>
<details>
<summary><b>Steps of Reproduction โ
</b></summary>
```mdx
1. Configure or use a native filter that renders `PluginFilterSelect`
(`superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx`)
with
`formData.inverseSelection` truthy; this sets the `inverseSelection`
variable from
`formData` at lines 160โ169.
2. When `PluginFilterSelect` renders, its return block at lines 54โ137
includes the
conditional `{inverseSelection && ( <style>โฆ </style> )}` at PR lines
575โ582; because
`inverseSelection` is true, React inserts a `<style>` element into the DOM
for that filter
instance with CSS targeting `.inverse-select-dropdown .ant-select-item {
margin-right:
14px; border-radius: 4px; }`.
3. The `Select` dropdown for this filter is given
`dropdownClassName`/`popupClassName="inverse-select-dropdown"` at PR lines
649โ651, so the
injected CSS correctly applies to its dropdown items; if multiple
inverse-selection
filters are present on a dashboard, each instance will render its own
identical `<style>`
tag.
4. In all these cases, the filters function correctly and styling is applied
as intended;
the only effect is multiple identical `<style>` nodes in the DOM (one per
inverse-selection filter instance), which modern browsers handle without
functional issues
but which represents minor, localized DOM/CSS duplication rather than a
correctness bug.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=80ddf1dda9ae4a2ba34e37279843298d&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=80ddf1dda9ae4a2ba34e37279843298d&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx
**Line:** 575:582
**Comment:**
*Performance: Rendering a raw `<style>` block inside the component
creates duplicate global style nodes for every inverse filter instance and on
remounts, which is an avoidable DOM/perf cost and makes styling harder to
control. Move these rules into existing styled-component CSS (scoped by
class/prop) so styles are emitted once and remain component-scoped.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40970&comment_hash=0e2e15eff1ffdc95c669de2929f9386972baa9a5d5970d30760f88eced897977&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40970&comment_hash=0e2e15eff1ffdc95c669de2929f9386972baa9a5d5970d30760f88eced897977&reaction=dislike'>๐</a>
##########
superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx:
##########
@@ -620,6 +646,8 @@ export default function PluginFilterSelect(props:
PluginFilterSelectProps) {
sortComparator={sortComparator}
onOpenChange={setFilterActive}
className="select-container"
+ dropdownClassName={inverseSelection ? "inverse-select-dropdown"
: undefined}
+ popupClassName={inverseSelection ? "inverse-select-dropdown" :
undefined}
Review Comment:
**Suggestion:** The custom `Select` wrapper in this codebase does not expose
`dropdownClassName`/`popupClassName` in its public `SelectProps` contract, so
this usage is an API mismatch and can break type-checked builds or future
refactors that stop forwarding unknown props. Add the supported popup-class
prop to the shared Select API (or use the officially supported prop only)
before using it here. [api mismatch]
<details>
<summary><b>Severity Level:</b> Critical ๐จ</summary>
```mdx
- โ Frontend TypeScript build fails on SelectFilterPlugin compilation.
- โ ๏ธ Native filter inverse-selection UI changes cannot ship while failing.
```
</details>
<details>
<summary><b>Steps of Reproduction โ
</b></summary>
```mdx
1. In
`superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx` the
filter
select control renders the shared `Select` component from
`@superset-ui/core/components`
with `dropdownClassName` and `popupClassName` props at PR lines 649โ650 (see
new hunk
where these props are added to `<Select ... />`).
2. The `Select` component used here is the wrapper defined in
`packages/superset-ui-core/src/components/Select/Select.tsx`, which is
exported via
`packages/superset-ui-core/src/components/Select/index.ts` and then
re-exported from
`packages/superset-ui-core/src/components/index.ts` (lines 191โ192), so
`<Select>` in the
plugin is typed with this wrapper's `SelectProps`.
3. The `SelectProps` type is defined in
`packages/superset-ui-core/src/components/Select/types.ts`:
`AntdExposedProps` (lines
42โ75) is a `Pick` of `AntdProps` that explicitly whitelists Ant Design
props such as
`allowClear`, `onOpenChange`, `getPopupContainer`, and `dropdownAlign`, but
does not
include `dropdownClassName` or `popupClassName`; `BaseSelectProps` (lines
79โ171) extends
`AntdExposedProps` and adds custom fields; `SelectProps` (lines 173โ185)
extends
`BaseSelectProps` without adding those popup-class props.
4. When you run the TypeScript type-check/build for `superset-frontend`
(which compiles
`SelectFilterPlugin.tsx` against the `@superset-ui/core` sources), the
compiler sees
`<Select ... dropdownClassName=... popupClassName=... />` where `Select` is
`forwardRef<SelectProps>` from `Select.tsx:100โ129`, and it will emit errors
like
"Property 'dropdownClassName' does not exist on type 'SelectProps'" and the
same for
`popupClassName`, causing the type-checked build to fail unless these props
are added to
`SelectProps` or the usage is suppressed.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=e4a503d3b7844d4f8f4e76ae73eaa000&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=e4a503d3b7844d4f8f4e76ae73eaa000&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx
**Line:** 649:650
**Comment:**
*Api Mismatch: The custom `Select` wrapper in this codebase does not
expose `dropdownClassName`/`popupClassName` in its public `SelectProps`
contract, so this usage is an API mismatch and can break type-checked builds or
future refactors that stop forwarding unknown props. Add the supported
popup-class prop to the shared Select API (or use the officially supported prop
only) before using it here.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40970&comment_hash=55daa0a29880c1a2a4dd2753141dceb37f09e7629f57469969a29e30cc726afa&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40970&comment_hash=55daa0a29880c1a2a4dd2753141dceb37f09e7629f57469969a29e30cc726afa&reaction=dislike'>๐</a>
--
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]
