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

Scrubbable Numeric Input #1528

Open
bengfarrell opened this issue Jun 10, 2021 · 7 comments · May be fixed by #4476
Open

Scrubbable Numeric Input #1528

bengfarrell opened this issue Jun 10, 2021 · 7 comments · May be fixed by #4476
Labels
a11y Issues related to accessibility Component: Number Field

Comments

@bengfarrell
Copy link
Collaborator

bengfarrell commented Jun 10, 2021

Expected Behaviour

  • Numeric input should offer an ability to turn on "scrubbing" or click and drag to increment/decrement
  • Also, a user should be able to hold a shift key down to accelerate the steps when using the arrow keys.

So, I could make a PR for this, but I think I'm too far ahead of the curve to even be under consideration against Spectrum design recommendations. I'm also messing with component and input element focus in ways that probably need more thought (that might impact FocusableBase maybe). It's also up to opinion whether the properties I created should act and are named the way I did them.

However, given that I thought more projects might want the feature, I thought I'd offer up a first pass, including storybook documentation as well.

For me, I'm just going to copy and paste into my project regardless, but maybe this'll help!

https://github.com/bengfarrell/spectrum-web-components

@Westbrook
Copy link
Contributor

Westbrook commented Jun 11, 2021

Modified Step

I really like this and would love to work out the right way to bring this into the library. Currently, we leverage a version of this in the Color elements. There we actually allow multiple modifier keys to multiply the multiplier: https://github.com/adobe/spectrum-web-components/blob/main/packages/color-area/src/ColorArea.ts#L212

Maybe that's overkill? Either way would that be a nice feature to abstract in a reusable manner across elements including the Number Field, standard Slider and Color Elements broadly?

While I'm thinking in this area, I also wonder if Home and End should move to the bounds of min/max while available? Something to consider separately, probably.

Could I convince you to submit a PR with this multiplier functionality? I think regardless of merging the nuances between this and other elements that we can likely adopt your approach here ASAP, even if it might not be 100% "Spectrum". Then we can refine its implementation and how it fits into the larger library over time.

Scrubbing

This is functionality that we are 100% planning to bring into the library, and I'd be quite happy to start doing so with a PR here as well: separate, preferably.

It's not immediately clear to me from reading the code how it works. Particularly, I see the scrub functionality stemming from the same listener as the stepper UI, which I will have to spend some more time with to fully understand. However, if it's working for you, I'd be quite happy to leverage this as the basis for the same in the main library.

One nuance question that I'll have to dig into the Spectrum docs for is whether you can scrub the input, or just the buttons. That may be overly restricting, in that the buttons are optional, but shouldn't be a big deal to move around as needed.

Another is you choice to hide scrubbing behind scrubbable. I'd love a little more information as to why you saw this as a requirement for this feature. It feels like if you can scrub somewhere, you'd expect to be able to do so everywhere, as a user.

Next steps

Thanks so much for starting the conversation here! Let me know what you think about submitting PRs. I'm not completely averse to just stealing code, but definitely prefer to ensure attribution is made to the right person when possible.

@bengfarrell
Copy link
Collaborator Author

Sounds good! I'll submit both separately tomorrow.

Modifier

So, in regards to the modified step, I'm definitely not against having multiple modifier keys active, but I really question the results here (in regards to my project anyway). It looks just like for every modifier key held down you run the delta calcs once more over. A potential 4x increase in speed but only if you're jamming on every possible modifier key at once. It doesn't seem the most configurable either. In our designs we're mainly using this for transform positions. So moving something by 4px vs 1px doesn't feel very impactful (and our designer wants 10px when shift is held).

TBH, I was even afraid to even use the shift key because in the original code you've blocked all input if any one of those modifiers are down. Now that I think the floodgates are open, I think I'll change it to any modifier to pair with the customized attribute to change the scale.

I am a bit confused by your "home" and "end" remarks. Just not sure what you're referring to.

Scrubbing

On scrubbing, yes - the pointer events took me a while to understand as well. But it seems to be a way to route all pointer events from a directive through to a change function, and the author who wrote it captures the change needed for the specific button as a function, and calls it at specific timed intervals. This is so a user can hold down the button and it keeps running the increment or decrement, but also is careful not to flood the system by calling it too often.

I was a bit uncertain what to do here, because it seemed silly to avoid the existing pointer event handlers and create my own, but obviously I don't want my handlers to repeat like they did either. So I loosely fit my solution to the existing, and was careful to make sure they don't go the same way.

In terms of only being able to scrub the buttons, I know in my app, we plan to hide the buttons all around. I'm not really seeing stepper buttons like these in any modern designs on my end. So if you decide to go down this route, I'd suggest you get in touch with a few designers working on the "you know what" teams. I think this would definitely be a point where my team would diverge from SWC if this decision was made.

Lastly, the scrubbable attribute. I'll happily remove it. I thought I was treading a bit on thin ice even suggesting this feature, which is why I put it behind a flag. I still question how good this will be with managing focus, since it is a bit different (no focus when scrubbing), so it breaks some of SWC's existing rules AFAIK about pointerdown on a component immediately receiving focus. So that was my other reason for putting it behind a flag.

@Westbrook
Copy link
Contributor

Modifiers

I gated those keys because at the specific implementation thereof it was less work to do that then to add the nuance of handling the specifics of when a modifier outlines that an arrow key move the carat to the beginning of the input as opposed to alters the value. I'll review more closely whether aria documentation prevents certain keys here from participating in this work, but I'd say you're relatively go to go. And, once we have an implementation that works we can edit it for any edge cases later.

Scrubber

Great points ass around. I just resurrected the next branch to catch "more complex changes" when they aren't fully baked. I'd love to get your implementation into something like that and then work from there, rather than completely make something from scratch. Once we have that we can discuss some easier changes in PR and some larger changes can be queued up from next before we fully adopt the feature.


Very excited to see these make their way into the library!

@Westbrook
Copy link
Contributor

For those interested, the following preview sites are up:

Thanks so much for sharing your code on this @bengfarrell!

@stanvladut
Copy link
Contributor

Great work on these very much requested features! 💯

Regarding the scrubbing functionality, our designers proposed something similar to this, where the input itself is left alone and the trigger for scrubbing to be the field's label:
https://user-images.githubusercontent.com/8905546/123397316-f4042100-d5aa-11eb-988d-0ee1486cae90.mov

I'm not really sure how this should work with inputs that have no label or some other corner cases, but I thought it's worth sharing this point of view and debate around it if valid.

@bengfarrell
Copy link
Collaborator Author

Great work on these very much requested features! 💯

Regarding the scrubbing functionality, our designers proposed something similar to this, where the input itself is left alone and the trigger for scrubbing to be the field's label:
https://user-images.githubusercontent.com/8905546/123397316-f4042100-d5aa-11eb-988d-0ee1486cae90.mov

I'm not really sure how this should work with inputs that have no label or some other corner cases, but I thought it's worth sharing this point of view and debate around it if valid.

That's an interesting idea! Ultimately, I don't feel like this is the right way to go not only for the reason that it may not have a label, but also the label might be on top or on the side and kind of an inconsistent experience in that regard. It probably also wouldn't be very discoverable as a feature on the label. I don't know that a user would ever think to themselves "Let me hover over this label and see if it's interactive", while the very action of using an input field means you have to hover over and click into it. On the way there, you'll see the new cursor.

Honestly, though, doing it other ways didn't cross my mind. Not that it appears to be there anymore (hope it comes back!), but my team really liked the functionality of the number field in XD and Illustrator, so I tried to model the interaction patterns after that because it worked.

thanks for the feedback! Happy to have my mind changed, of course!

@najikahalsema
Copy link
Collaborator

needs a11y and Spectrum approval before we can ship this.

@Westbrook Westbrook added the a11y Issues related to accessibility label Jun 2, 2023
@Westbrook Westbrook linked a pull request May 21, 2024 that will close this issue
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a11y Issues related to accessibility Component: Number Field
Projects
Development

Successfully merging a pull request may close this issue.

4 participants