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

[Feature Request]: Provide an easy way to disable wheel events in NumberInput #11982

Closed
1 task done
caasi opened this issue Aug 21, 2022 · 3 comments · Fixed by #12358
Closed
1 task done

[Feature Request]: Provide an easy way to disable wheel events in NumberInput #11982

caasi opened this issue Aug 21, 2022 · 3 comments · Fixed by #12358
Labels
component: number-input good first issue 👋 Used by GitHub to elevate contribution opportunities hacktoberfest See https://hacktoberfest.com/ needs: community contribution Due to roadmap and resource availability, we are looking for outside contributions on this issue. proposal: accepted This request has gone through triaging and we are accepting PR's against it. type: enhancement 💡

Comments

@caasi
Copy link

caasi commented Aug 21, 2022

Summary

If I want to disable wheel events in NumberInput, I have to add an event listener manually.

Could we have an easier way to disable wheel events in NumberInput?

function MyNumberInput = () => {
  return (
    <NumberInput
      {...rest}
      ref={inputRef}
      onFocus={() => {
        if (inputRef.current) {
          inputRef.current.addEventListener("wheel", disableWheel);
        }
      }}
      onBlur={() => {
        if (inputRef.current) {
          inputRef.current.removeEventListener("wheel", disableWheel);
        }
      }}
    />
  );
};

/**
 * It prevents any wheel event from emitting.
 *
 * We want to prevent this input field from changing by the user accidentally
 * when the user scrolling up or down in a long form. So we prevent the default
 * behavior of wheel events when it is focused.
 *
 * Because React uses passive event handler by default, we can't just call
 * `preventDefault` in the `onWheel` event handler. So we have to get the input
 * ref and add our event handler manually.
 *
 * @see https://github.com/facebook/react/pull/19654
 * @param {WheelEvent} e A wheel event.
 */
function disableWheel(e) {
  e.preventDefault();
}

Justification

When user editing a long form with a number input field, it is very easy to change the number input accidentally. User complains about saving the wrong value multiple times.

Desired UX and success metrics

No response

Required functionality

No response

Specific timeline issues / requests

No response

Your team

@Superbil

Available extra resources

https://stackoverflow.com/questions/9712295/disable-scrolling-on-input-type-number

Code of Conduct

@tw15egan tw15egan added proposal: open This request has gone through triaging. We're determining whether we take this on or not. component: number-input and removed status: needs triage 🕵️‍♀️ labels Aug 22, 2022
@tay1orjones
Copy link
Member

tay1orjones commented Aug 22, 2022

Yeah, due to React not supporting passive event listeners, facebook/react#6436, this has frustrating ergonomics having to apply the handler to the ref like this

I think this could be baked into the component behind a new disableWheel prop.

It's important to point out your example is correct - the handler should be added only on focus, and removed on blur.

I'll add it to our backlog. It's unlikely we'll be able to tackle this anytime soon, but would gladly accept a PR if you're up for submitting this change @caasi 🙏

@tay1orjones tay1orjones added proposal: accepted This request has gone through triaging and we are accepting PR's against it. and removed proposal: open This request has gone through triaging. We're determining whether we take this on or not. labels Aug 22, 2022
@caasi
Copy link
Author

caasi commented Aug 24, 2022

@tay1orjones Thank you. And thanks to Scotty H on Stack Overflow for providing his jQuery solution.

I can't promise to issue a PR right now. So it's OK to me to leave this open.

@mgueyraud
Copy link
Contributor

Hi guys!, I've created a PR for this issue (#12358), let me know if the approach is good!.

@sstrubberg sstrubberg added the hacktoberfest See https://hacktoberfest.com/ label Oct 21, 2022
@kodiakhq kodiakhq bot closed this as completed in #12358 Oct 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: number-input good first issue 👋 Used by GitHub to elevate contribution opportunities hacktoberfest See https://hacktoberfest.com/ needs: community contribution Due to roadmap and resource availability, we are looking for outside contributions on this issue. proposal: accepted This request has gone through triaging and we are accepting PR's against it. type: enhancement 💡
Projects
Archived in project
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants