codeant-ai-for-open-source[bot] commented on code in PR #42600:
URL: https://github.com/apache/superset/pull/42600#discussion_r3679185998
##########
superset-frontend/packages/superset-ui-core/src/components/Select/Select.stories.tsx:
##########
@@ -338,6 +338,46 @@ AtEveryCorner.parameters = {
},
};
+const manyPermissionLabels = [
+ 'can read SavedQuery',
+ 'can write SavedQuery',
+ 'can read Database',
+ 'can read Query',
+ 'can activate TabStateView',
+ 'can copy clipboard Superset',
+ 'can csv Superset',
+ 'can delete query TabStateView',
+ 'can delete TableSchemaView',
+ 'can delete TabStateView',
+ 'can estimate query cost SQLLab',
+ 'can execute sql query SQLLab',
+];
+const manyPermissionOptions = Array.from({ length: 32 }, (_, i) => ({
+ label:
+ manyPermissionLabels[i % manyPermissionLabels.length] +
+ (i >= manyPermissionLabels.length ? ` ${i}` : ''),
+ value: `perm_${i}`,
+}));
+
+/**
+ * Regression coverage for a narrow container where selected tags wrap to
+ * multiple rows (e.g. the Role edit modal's Permissions field, which has no
+ * `maxTagCount` override and defaults to wrapping at 4 tags per row). See
+ * https://github.com/apache/superset/issues/39339.
+ */
+export const ManySelectedValuesWrapping = () => (
+ <div style={{ width: 600 }}>
+ <Select
+ ariaLabel="many-selected-values-select"
+ mode="multiple"
+ showSearch
+ options={manyPermissionOptions}
+ value={manyPermissionOptions.map(o => o.value)}
+ getPopupContainer={trigger => trigger.parentElement as HTMLElement}
Review Comment:
**Suggestion:** This story does not actually display all 32 selected tags:
the `Select` implementation supplies `MAX_TAG_COUNT` (4) when `maxTagCount` is
omitted, so Ant Design collapses the selection to four tags plus an overflow
tag instead of rendering wrapped rows. Consequently, the story cannot
demonstrate the multi-row container regression described in its comment. Pass a
tag-count configuration that allows the selected tags to render across multiple
rows and verify the resulting layout. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Storybook regression scenario does not display wrapped tags.
- ⚠️ Visual verification cannot reproduce the reported multi-row overflow.
- ⚠️ Future Select layout regressions may go unnoticed.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=fa49d6f195984807bd789a1a9c501867&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=fa49d6f195984807bd789a1a9c501867&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/packages/superset-ui-core/src/components/Select/Select.stories.tsx
**Line:** 363:376
**Comment:**
*Api Mismatch: This story does not actually display all 32 selected
tags: the `Select` implementation supplies `MAX_TAG_COUNT` (4) when
`maxTagCount` is omitted, so Ant Design collapses the selection to four tags
plus an overflow tag instead of rendering wrapped rows. Consequently, the story
cannot demonstrate the multi-row container regression described in its comment.
Pass a tag-count configuration that allows the selected tags to render across
multiple rows and verify the resulting layout.
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%2F42600&comment_hash=4b61eb932a123c73598f2a461e3004dd77a8b7d0c626afd4fe0c4706d5198234&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42600&comment_hash=4b61eb932a123c73598f2a461e3004dd77a8b7d0c626afd4fe0c4706d5198234&reaction=dislike'>👎</a>
##########
superset-frontend/packages/superset-ui-core/src/components/Select/Select.test.tsx:
##########
@@ -898,6 +898,28 @@ test('Renders only an overflow tag if dropdown is open in
oneLine mode', async (
expect(withinSelector.getByText('+ 2 ...')).toBeVisible();
});
+// Regression test for the bug described in:
https://github.com/apache/superset/issues/39339
+// The AntD v6 upgrade renamed `.ant-select-selection-overflow` (the tag
+// container) to `.ant-select-content`, and a CSS rule meant only to cap the
+// height of individual tags/placeholder text was mistakenly widened to also
+// match the renamed container class. That capped the whole multi-row tag
+// container to a single line's height, so wrapped rows spilled outside the
+// select's border, the dropdown arrow mis-centered against the wrong height,
+// and the search input (rendered last in the wrapped row) became invisible
+// once the dropdown opened.
+test('does not cap the tag container to a single line when tags wrap to
multiple rows', () => {
+ render(
+ <Select
+ {...defaultProps}
+ value={OPTIONS.slice(0, 8)}
+ mode="multiple"
+ maxTagCount={6}
+ />,
+ );
+ const content = getElementByClassName('.ant-select-content');
+ expect(content).not.toHaveStyle({ maxHeight: '32px' });
Review Comment:
**Suggestion:** The test does not reliably exercise the multi-row layout it
claims to cover: `maxTagCount={6}` causes Ant Design to replace the remaining
selected values with an overflow tag, and the test does not constrain the
select width. As a result, the tags may never wrap, so this assertion can pass
without detecting a regression in the container height. Render the select in a
narrow-width wrapper and configure it to display all selected tags (or
otherwise verify that multiple tag rows are present) before asserting the
container style. [possible bug]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Select regression test may pass without exercising wrapping.
- ⚠️ Multi-row container height behavior remains insufficiently verified.
- ⚠️ Future CSS regressions could evade automated coverage.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=7cc653f39fc3465eb48619ca22302059&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=7cc653f39fc3465eb48619ca22302059&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/packages/superset-ui-core/src/components/Select/Select.test.tsx
**Line:** 910:920
**Comment:**
*Possible Bug: The test does not reliably exercise the multi-row layout
it claims to cover: `maxTagCount={6}` causes Ant Design to replace the
remaining selected values with an overflow tag, and the test does not constrain
the select width. As a result, the tags may never wrap, so this assertion can
pass without detecting a regression in the container height. Render the select
in a narrow-width wrapper and configure it to display all selected tags (or
otherwise verify that multiple tag rows are present) before asserting the
container style.
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%2F42600&comment_hash=6a3b0235a91f80ac1d9c07689171fe347ea7c0960b65a5256dcb97e49f89720d&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42600&comment_hash=6a3b0235a91f80ac1d9c07689171fe347ea7c0960b65a5256dcb97e49f89720d&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]
