Repository navigation
fix(theme): match outlined Select height to TextField at 56px - #1878
Conversation
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Docstring Coverage |  |
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Description Check | |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title clearly describes the main change: aligning outlined Select height with TextField at 56px through the MuiOutlinedInput line-height update and design token changes. |
Full details: Docstring Coverage
-
Explanation
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
…ld height textB1Regular's 1.75rem body line-height was spread onto the MuiOutlinedInput root. Select sizes its display div from the inherited line-height while the TextField input has a fixed 1.4375em height, so outlined Selects rendered 61px (small: 45px) against 56px (small: 40px) TextFields. Restore MUI's InputBase line-height on the root and the sm breakpoint. Signed-off-by: Cooper Fitzgerald <182300328+fitzergerald@users.noreply.github.com>
Signed-off-by: Cooper Fitzgerald <182300328+fitzergerald@users.noreply.github.com>
d77ca12 to
fdd4c50
Compare
Visual proofTextField / Select / Autocomplete side by side, every variant and size, measured as the
Consumer note: multiline outlined TextFields now use MUI's default 1.4375em line spacing instead of 1.75rem, so a |
3 participants
Intent
When end-to-end testing, be picky about the UI you see and be obsessed with pixel perfection. Adhere to and honor the design system. If something looks off, even if it is not directly related to what you are doing, try to get it fixed along the way. If the design system is not being followed, flag it and get it fixed.
With the Sistent theme, Select-based fields render 61px tall while TextField renders 56px, so forms that mix them misalign.
What Changed
MuiOutlinedInputroot (src/theme/components/outlinedinput.modifier.ts) now resetslineHeightto MUI's InputBase value1.4375emafter spreadingtextB1Regular. It does this at the root and again at thesmbreakpoint, where it also keeps the rest of the typography'ssmoverrides. Before this, the 1.75rem body line-height reached the field and made outlined Selects 61px tall while TextFields were 56px.theme.test.tscase that checks the1.4375emline-height in light and dark mode, at both the root and thesmbreakpoint.DESIGN.mdnow setsheight: 56pxon theinput-fieldandinput-field-darkcomponent tokens.Risk Assessment
✅ Low: This is a small, theme-scoped CSS override that restores MUI's own InputBase line-height for outlined fields. The open sibling (the standard variant) and the multiline side effect are visual only and can be fixed as follow-ups.
Testing
I built disposable base and head harness pages that render the real Sistent theme provider and fields, then drove them in headless Chromium in light and dark mode at 1280px and 420px. On base, all three Select forms (bare, FormControl, TextField select) were 61/45px while TextField and Autocomplete were 56/40px. On head, every single-line field is 56/40px with a 23px line-height. Before/after screenshots and the measured heights are saved as evidence. A Select still opens its menu normally on head, and the focused theme unit test passes. The multiline TextField gets shorter (89 to 79px). That is the already-accepted, MUI-default side effect. The harness, browser and server were torn down, and the worktree is clean.
Evidence: Measured field heights, base vs head, all modes and widths
base medium: textfield 56, select 61, formcontrolselect 61, selectprop 61, autocomplete 56 | small: 40/45/45/45/40 head medium: textfield 56, select 56, formcontrolselect 56, selectprop 56, autocomplete 56 | small: 40/40/40/40/40 (same in light/dark, 1280/420)Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ No issues found.
src/theme/components/input.modifier.ts:13- The same bug is still present for the standard (underlined) variant.MuiInput.styleOverrides.rootspreadstextB1Regular(lineHeight 1.75rem = 28px) and does not restore MUI InputBase'slineHeight: '1.4375em'. A Select's display div gets its height from the inherited line-height, but a TextField<input>has a fixed 1.4375em height. So<Select variant="standard">still renders 5px taller than<TextField variant="standard">. That is the exact 61-vs-56 mismatch this change fixes for outlined fields, moved to the underlined variant. In-repo standard-variant consumers: src/custom/DashboardWidgets/WorkspaceActivityWidget.tsx:133 (standard Select) and src/custom/SearchBar.tsx:149 (standard TextField). Fix: apply the samelineHeight: FIELD_LINE_HEIGHToverride, plus the sm-breakpoint override, in MuiInput, and share the constant from one module so both stay in sync. Extend the theme.test.ts case to cover MuiInput too.src/theme/components/outlinedinput.modifier.ts:27- Side effect consumers will see: MUI's InputBase input/textarea usesfont: inherit, so multiline outlined TextFields now inherit 1.4375em (about 23px) per line instead of 1.75rem (28px). A rows={4} textarea gets about 20px shorter, with tighter line spacing. This matches MUI's own default, and it also makes a single-row multiline field 56px. Mention it in the PR description so downstream UIs (Meshery, Kanvas) expect the visual change.🔧 **Test** - 1 issue found ✅
src/theme/components/outlinedinput.modifier.ts:27- Side effect of the fix: the multiline outlined TextField also inherits the new 1.4375em line-height, so its lines are 23px apart instead of 28px and a 2-row field shrinks from 89px to 79px. That matches MUI's default and renders cleanly. If the 28px body spacing was intentional for textareas, it could be restored on the multiline input alone.Bundled a disposable test page with esbuild against the worktreesrc/(SistentThemeProvider plus Sistent's TextField, Select, Autocomplete, native Select and multiline TextField, each in medium and small sizes). Built it twice: once with the head theme and once with the base commit'soutlinedinput.modifier.tsswapped in.Drove both builds in headless Chromium (playwright-core) at 1280px and 420px widths (420px is below the sm breakpoint), in light and dark mode. Measured the.MuiOutlinedInput-rootheights, computed line-heights and scroll versus client heights, and took full-page screenshots.npx jest src/__testing__/theme.test.ts✅ No issues found.
Built disposable esbuild harness pages from base (b2d92173) and head (git archive) src, each renderingSistentThemeProviderwith TextField, bare Select, FormControl+InputLabel+Select, Autocomplete, multiline TextField andTextField selectat medium and small sizesServed the pages on a local port and drove them in headless Chromium throughchrome-devtools-axi(CHROME_DEVTOOLS_AXI_BROWSER_URL pointing at a throwaway profile)Measured each.MuiInputBase-rootheight and computed line-height viachrome-devtools-axi evalfor {base, head} x {light, dark} x {1280px, 420px}Took full-page screenshots of all 8 combinationsClicked a Select on head, confirmed the listbox options appear, and took a screenshot of the open menunpx jest src/__testing__/theme.test.ts -t "line-height"(2 passed)✅ **Document** - passed
✅ No issues found.
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ No issues found.
✅ **Push** - passed
✅ No issues found.
✅ No issues found.