-
Notifications
You must be signed in to change notification settings - Fork 15
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
[MP-345] Restore number input behaviour and look #4619
Conversation
🦋 Changeset detectedLatest commit: 3ab7599 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
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 |
bc71caa
to
617d238
Compare
@toptal-bot run package:alpha-release |
Your alpha package is ready 🎉 |
@toptal-anvil ping reviewers |
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.
LGTM 👍
MP-345
Why is it important
Without this fix, portals can not be updated to latest Picasso due to visual and form submission flow changes.
After this fix, the Client Portal has all checks passing https://github.com/toptal/client-portal/pull/9995 ✅
Description
When
NumberInput
is used from corresponding form control, the form control passesmin
andmax
(after #4610) toNumberInput
.NumberInput
passesmin
andmax
further to actual number HTML input which leads tomin
andmax
values (https://stackoverflow.com/questions/33283901/input-number-max-attribute-resizes-field)Behaviour introduced in this pull request
This pull request gets back to the original behaviour when https://github.com/toptal/picasso/tree/master/packages/base/NumberInput had empty (or invalid, which has the same effect)
min
andmax
attributes that was preventing browser from modifying the look and behaviour of the component.How to test
min
andmax
values with differentstep
Screenshots
Before
Kapture.2024-11-05.at.14.48.25.mp4
After
Kapture.2024-11-05.at.14.47.08.mp4
Development checks
picasso-tailwind-merge
requires major update (check itsREADME.md
)props
in component with documentationexamples
for componentPR commands
List of available commands:
@toptal-bot run package:alpha-release
- Release alpha version@toptal-anvil ping reviewers
- Ping FX team for reviewPR Review Guidelines
When to approve? ✅
You are OK with merging this PR and
nit:
to your comment. (ex.nit: I'd rename this variable from makeCircle to getCircle
)When to request changes? ❌
You are not OK with merging this PR because
When to comment (neither ✅ nor ❌)
You want your comments to be addressed before merging this PR in cases like:
How to handle the comments?