-
Notifications
You must be signed in to change notification settings - Fork 73
[LG-5863] fix(NumberInput): align styles with design #3453
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.
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
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: ccfb6b2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Drive-by fix to improve prop selection
| ref={inputRef} | ||
| value={value} | ||
| unit={unitOptions.length ? unit : unitProp} | ||
| unit={unitOptions.length ? unit : ((unitProp ?? 'one') as string)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Drive-by to fix type error
|
Note to reviewer: By removing uppercase enforcement this PR can create a scenario where a consumer had a value "month" that was being rendered "MONTH" and will now just be rendered "month." I considered enforcing capitalization, but I thought this wasn't a horrible misalignment and was easy enough to fix (hopefully) on upgrade. Would appreciate a second opinion on this! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR aligns the NumberInput component's styling with design specifications by removing forced uppercase text transformation and standardizing font weights.
Changes:
- Removed uppercase text transformation from unit select buttons
- Updated font weight to semiBold across unit display elements
- Changed unit-only display from
OverlinetoBodytypography component
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/number-input/src/UnitSelectButton/UnitSelectButton.styles.ts | Removes uppercase text transformation from menu button |
| packages/number-input/src/UnitSelect/UnitSelect.styles.ts | Adds semiBold font weight and removes text transformation |
| packages/number-input/src/NumberInput/NumberInput.tsx | Replaces Overline with Body component for unit display |
| packages/number-input/src/NumberInput/NumberInput.styles.ts | Adds semiBold font weight to unit styles |
| packages/number-input/src/NumberInput.stories.tsx | Updates story display names and fixes type handling |
| packages/select/src/Select.stories.tsx | Adds state, size, and message controls to story |
| .changeset/fruity-candles-march.md | Documents the style changes |
|
Size Change: +22 B (0%) Total Size: 1.83 MB
ℹ️ View Unchanged
|
|
Coverage after merging LG-5863/num-input-no-caps into main will be
Coverage Report for Changed Files
|
|||||||||||||||||||
✍️ Proposed changes
Drive-by's:
NumberInputstory type issueSelectstory🎟 Jira ticket: LG-5863
✅ Checklist
pnpm changesetand documented my changes🧪 How to test changes
Verify in Storybook
📷 Screenshots
Before
After