Conversation
6992248 to
023fe9d
Compare
585d357 to
c0f561a
Compare
|
Rebased on HEAD. It seems to fix Circle CI, I don't know why. |
71b21fe to
4aa63f6
Compare
|
It might be subjective, but the scrubber feels too sensitive. Changing value on every pixel of pointer movement may be too fast. Figma seems to update the value every two pixels (or perhaps they take pixel density into consideration?). We could make this configurable by adding the |
|
@michaldudak yeah you're right, previously it was 2px but I must have changed something that reverted it back to 1px. Will fix. |
colmtuite
left a comment
There was a problem hiding this comment.
Tested on Chrome, Safari, Safari iOS, and Chrome iOS. Feedback/thoughts so far:
ScrubAreaCursoris missing from the docs anatomy section.- Seems like we don't have the teleport threshold thingy prop on
ScrubAreaCursoryet? Are you planning to add that? - Should scrubbing start from where the pointer was when scrubbing began, or when the pointer direction changes during scrubbing? Idk how to explain wtf I'm on about here 🤣 It's noticeable when
minandmaxare set. If you setmax, then scrub way beyond themaxvalue, you need to scrub all the way back down before the value decrements. Perhaps it could begin decrementing immediately when the pointer direction changes? - The
largeSteplogic seems odd to me. I expected it to just increment/decrement the value by whatever thelargeStepvalue is? smallStepdoesn't seem to be working for me. Nothing happens when I holdcmdand then increment/decrement.
Still to test: formatting options.
99d4bb0 to
7866e25
Compare
|
Decisions from a recent call:
|
Can you elaborate more here? The We might be missing props similar to |
Ok yeah my first thought was
No this was just a note I took from our call, not sure what you meant when you were talking about formatting, internationalisation, and functions. I remember you saying some piece of work around internationalisation is still incomplete? |
Alright, will change 👍
Yeah there were some bugs remaining with the parsing and validation functions, but I should have fixed most of them in the recent commits. However these are only internal and not part of any API. |
| return ( | ||
| <NumberField value={value} onChange={setValue}> | ||
| <NumberField.Group> | ||
| <NumberField.Decrement>−</NumberField.Decrement> |
There was a problem hiding this comment.
&minus is wider and looks more in proportion with + 😅
| import { mergeReactProps } from '../utils/mergeReactProps'; | ||
|
|
||
| /** | ||
| * @ignore - internal hook. |
There was a problem hiding this comment.
Don't we want to expose it like the other hooks?
There was a problem hiding this comment.
No as it's just an internal hook to place some of the functionality in its own hook
Bifurcate tests Docs stylelint Attempt test fix Ignore perf test in JSDOM Use skip Docs Format Update docs Relocate ScrubArea logic Memo return value Unit tests for parsing and formatting Skip test in browser Tests Relocate pointerDownEvent Remove floating point errors Docs Update allowed chars Bug fixes and feedback Remove autoFocus destructure Remove clamp requirement logic Allow currency symbols Move context menu handler Avoid calling handlers while disabled Unsubscribe from visualViewport Add description to useNumberField Docs Use mergeReactProps Use altKey for smallStep Fix cursor rendering
75cf7b3 to
c8cc076
Compare
c618708 to
1c231b4
Compare
Closes #185
Closes #224
Preview: https://deploy-preview-186--base-ui.netlify.app/base-ui/react-number-field/
TODO:
teleportDistance(or other name) prop to loop closer to the input compared to the viewport.displayNameto Contexts.Intl.NumberFormatusage.inputMode="numeric"prop for iOS when appropriate, which doesn't show a negative sign in the software keyboard, it must be "text".inputModeis "text".ScrubArealogic into theuseNumberFieldhook, or possibly a separate hook. Depends how granular the tree-shaking could possibly be for this functionality.