Skip to content
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

Chore/add field styles for number input #823

Merged
merged 4 commits into from
Dec 16, 2024

Conversation

joyce-foo
Copy link
Contributor

@joyce-foo joyce-foo commented Dec 16, 2024

Currently NumberInput does not allow for overriding of inner input styles. Armoury needs to override the padding so that the number does not cut off
image

Solution
Allow merging of styles for inner input component in NumberInput

Local test
Screenshot 2024-12-16 at 5 43 31 PM

@joyce-foo joyce-foo requested a review from karrui December 16, 2024 09:47
Comment on lines 58 to 61
const mergedInputStyles = useMemo(
() => mergeThemeOverride(styles.field, inputStyles),
[inputStyles, styles.field],
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

might not need to use mergeThemeOverride and just assign inputStyles directly to input's sx prop. Chakra UI has a priority list for styles (_css < sx). Can you try and see whether just using sx with inputStyles is enough?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you're right, it works. Just updated

@karrui karrui merged commit 0f307e2 into main Dec 16, 2024
12 checks passed
@karrui karrui deleted the chore/add-field-styles-for-number-input branch December 16, 2024 10:08
@karrui
Copy link
Collaborator

karrui commented Dec 16, 2024

I'll run a release tomorrow morning. ping me if i forget @joyce-foo

@karrui karrui mentioned this pull request Dec 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants