Skip to content

fix(native-filters): keep filter value input caret left-aligned #42266

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
fitzee wants to merge 1 commit into from
Closed

fix(native-filters): keep filter value input caret left-aligned #42266

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions superset-frontend/src/filters/components/common.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,22 @@ export const RESPONSIVE_WIDTH = 0;
export const FilterPluginStyle = styled.div<PluginFilterStylesProps>`
min-height: ${({ height }) => height}px;
width: ${({ width }) => (width === RESPONSIVE_WIDTH ? '100%' : `${width}px`)};
/* Input / InputNumber filter value controls resolve text-align by */
/* inheritance, so pin them to the inline start to keep the caret and typed */
/* text at the left edge. */
Comment on lines +28 to +30

@codeant-ai-for-open-source codeant-ai-for-open-source Bot Jul 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The new comment says text is kept at the “left edge,” but the implemented rule uses text-align: start, which maps to the right edge in RTL; update the comment to describe “inline start” instead to avoid misleading future changes and tests. [comment mismatch]

Severity Level: Minor 🧹 
- ⚠️ Comment misleading about RTL alignment of filter inputs.
- ⚠️ No runtime behavior affected; UI works as intended.
Steps of Reproduction ✅ 
1. Open `superset-frontend/src/filters/components/common.ts` and locate
`FilterPluginStyle` definition at lines 25-44 in the PR diff.

2. Observe the comments at lines 28-30: they mention "inline start" (correct for
`text-align: start`) but also say "text at the left edge."

3. Note the implementation at line 31: `text-align: start;` which, per CSS spec, aligns
text to the right edge in RTL contexts.

4. In an RTL dashboard (where inline start is right), native filter value inputs render
caret/text at the right edge, while the comment still describes "left edge", so the
comment is mildly misleading but behavior is correct.

Fix in Cursor Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** superset-frontend/src/filters/components/common.ts
**Line:** 28:30
**Comment:**
	*Comment Mismatch: The new comment says text is kept at the “left edge,” but the implemented rule uses `text-align: start`, which maps to the right edge in RTL; update the comment to describe “inline start” instead to avoid misleading future changes and tests.

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
👍 | 👎

Comment on lines +28 to +30

@yousoph yousoph Jul 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reword to avoid the physical-"left" framing (RTL-correct) and cover all three value controls:

Suggested change
/* Input / InputNumber filter value controls resolve text-align by */
/* inheritance, so pin them to the inline start to keep the caret and typed */
/* text at the left edge. */
/* Filter value controls (Input / InputNumber / Select search) resolve */
/* 'text-align' by inheritance, so pin them to the inline start to keep the */
/* caret and typed text at the inline-start edge. */

text-align: start;

@sadpandajoe sadpandajoe Jul 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a focused Jest regression test for this fix? Rendering FilterPluginStyle and asserting each retained text-align: start rule with toHaveStyleRule would protect the corrective CSS; the existing DOM-contract test only protects the Ant Design class hook.


/* antd v6 sets 'text-align: center' directly on '.ant-select-content' for */

@sadpandajoe sadpandajoe Jul 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The locked Ant Design 6.5.1 CSS does not set text-align: center on .ant-select-multiple .ant-select-content; that rule only sets wrapping, item alignment, and line height. Can we keep the wrapper-level rule, remove this nested selector, and document the actual inherited source instead?

/* multiple-mode Selects (its selector '.ant-select-multiple */
/* .ant-select-content' has specificity 0,2,0). The Select search input */
/* lives inside '.ant-select-content' and inherits that value, so the caret */
/* and typed text render centered. A value inherited from this wrapper */
/* cannot beat a rule that targets '.ant-select-content' directly, so */
/* override the element itself: this wrapper class plus the two antd classes */
/* give specificity 0,3,0, which wins without !important. RTL-safe. */
Comment on lines +33 to +40

@yousoph yousoph Jul 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stock antd 6.0.0–6.5.1 doesn't set text-align: center on .ant-select-content (only on .ant-select-clear), so this rewording drops the inaccurate "antd v6 sets…" claim while keeping the specificity reasoning, which is correct:

Suggested change
/* antd v6 sets 'text-align: center' directly on '.ant-select-content' for */
/* multiple-mode Selects (its selector '.ant-select-multiple */
/* .ant-select-content' has specificity 0,2,0). The Select search input */
/* lives inside '.ant-select-content' and inherits that value, so the caret */
/* and typed text render centered. A value inherited from this wrapper */
/* cannot beat a rule that targets '.ant-select-content' directly, so */
/* override the element itself: this wrapper class plus the two antd classes */
/* give specificity 0,3,0, which wins without !important. RTL-safe. */
/* Some deployments (custom themes / css-in-js overrides) can leak a centered */
/* 'text-align' onto '.ant-select-content'. The Select search <input> lives */
/* inside '.ant-select-content' and resolves 'text-align' by inheritance, so */
/* that would center the caret and typed text. Overriding '.ant-select-content' */
/* directly (this wrapper class + two antd classes = specificity 0,3,0) beats */
/* both an inherited value and a '.ant-select-multiple .ant-select-content' */
/* rule without '!important'. 'start' is the CSS default, so this is a no-op */
/* unless a centered value leaked in, and it's RTL-safe. */

.ant-select .ant-select-content {
text-align: start;
}
`;

export const StyledFormItem = styled(FormItem)`
Expand Down
Loading