codeant-ai-for-open-source[bot] commented on code in PR #40970:
URL: https://github.com/apache/superset/pull/40970#discussion_r3418372456
##########
superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx:
##########
@@ -102,14 +102,32 @@ const StyledSpace = styled(Space)<{
width: 100%;
.exclude-select {
- width: 80px;
- flex-shrink: 0;
+ width: 100%;
+ min-width: 80px;
}
&.ant-space {
.ant-space-item {
width: ${({ inverseSelection }) => (!inverseSelection ? '100%' :
'auto')};
}
+
+ ${({ inverseSelection }) =>
+ inverseSelection &&
+ `
+ .ant-space-item:first-of-type {
+ flex: 1 1 25%;
+ }
+
+ .ant-space-item:last-child {
+ flex: 3 1 75%;
+ min-width: 0;
+ }
+
+ .select-container {
+ width: 100% !important;
+ max-width: none !important;
+ }
+ `}
Review Comment:
**Suggestion:** The inverse-selection flex layout is applied whenever
`inverseSelection` is true, but in Filter Config Modal the exclude selector is
not rendered, so there is only one `Space` item. In that case the same item
matches both `:first-of-type` and `:last-child`, forcing a 75% flex basis and
causing the main input/select to not use full width. Gate this flex split logic
by `appSection !== AppSection.FilterConfigModal` (or by checking there are two
items) so the single control remains 100% width. [css layout issue]
<details>
<summary><b>Severity Level:</b> Minor ๐งน</summary>
```mdx
- โ ๏ธ Filter Config Modal default-value preview select not full width.
- โ ๏ธ Inverse-selection configuration harder to read in modal preview.
```
</details>
<details>
<summary><b>Steps of Reproduction โ
</b></summary>
```mdx
1. Open the Filters Config Modal, which renders the default value preview via
`DefaultValue` at
`superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/DefaultValue.tsx:20-35`,
where `SuperChart` is invoked with
`appSection={AppSection.FilterConfigModal}` (line 23).
2. In that modal, configure a filter that uses the Select filter plugin and
enable
`inverseSelection` in the controls (field is part of `controlsOrder`
including
`'inverseSelection'` at `FiltersConfigForm.tsx:10-17`), so
`formData.inverseSelection` is
`true` and passed through to the select plugin.
3. The select plugin component `PluginFilterSelect` (exported from
`superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx`)
renders
`StyledSpace` around its controls at lines 55-63 (absolute ~580-583), passing
`appSection={appSection}` and `inverseSelection={inverseSelection}`; because
`appSection
=== AppSection.FilterConfigModal`, the leading exclude `<Select
className="exclude-select"
... />` is not rendered due to the condition `{appSection !==
AppSection.FilterConfigModal
&& inverseSelection && (...)}` at lines 64-73, so AntD `Space` wraps only a
single child
(the input/select).
4. With `inverseSelection` true and a single child, the single
`.ant-space-item` generated
by AntD `Space` matches both `.ant-space-item:first-of-type` and
`.ant-space-item:last-child` in `StyledSpace`'s CSS at
`SelectFilterPlugin.tsx:117-123`,
so the two-column flex split (`flex: 1 1 25%` then `flex: 3 1 75%`) intended
for two items
is applied to this single item; since this layout is not gated by
`appSection`, the Filter
Config Modal default-value control uses the same split, reducing its
effective flex basis
compared to the normal filter bar and causing the control in the modal to
not occupy the
full available width.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=05b8234416384281b76f333f55b3954e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=05b8234416384281b76f333f55b3954e&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:** 114:130
**Comment:**
*Css Layout Issue: The inverse-selection flex layout is applied
whenever `inverseSelection` is true, but in Filter Config Modal the exclude
selector is not rendered, so there is only one `Space` item. In that case the
same item matches both `:first-of-type` and `:last-child`, forcing a 75% flex
basis and causing the main input/select to not use full width. Gate this flex
split logic by `appSection !== AppSection.FilterConfigModal` (or by checking
there are two items) so the single control remains 100% width.
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=9f2f89e11b037765900fe4aa5d788a3159aa8fedfc47b6905d3fe51ecb7347f2&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40970&comment_hash=9f2f89e11b037765900fe4aa5d788a3159aa8fedfc47b6905d3fe51ecb7347f2&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]
