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

fix: remove input type from number field #7025

Merged
merged 3 commits into from
Jun 14, 2023
Merged

Conversation

maribethb
Copy link
Contributor

@maribethb maribethb commented Apr 28, 2023

The basics

  • I branched from develop
  • My pull request is against develop
  • My code follows the style guide
  • I ran npm run format and npm run lint

The details

Resolves

Fixes an issue with number fields where the browser doesn't let you enter text https://groups.google.com/g/blockly/c/qksb1qT54Ds

This prevents you from entering hexadecimal numbers or Infinity

Proposed changes

Remove input type=number so that users can enter Infinity and hexadecimal values, including on mobile.

Alternatives Considered

Instead of using input type=number use inputMode=numeric. This provides a hint to mobile browsers to show a number keyboard but doesn't enforce the "no text" rule. However, on Android, the keyboard cannot be switched back to the alpha keyboard. There is no setting that matches "This input is normally a number, but still allow the user to type text" so it is simply not possible. See #1595 for more information.

https://css-tricks.com/everything-you-ever-wanted-to-know-about-inputmode/#aa-numeric

Behavior Before Change

  • Can only enter numbers. any letters will not even be accepted by the browser to be passed to the validator and checked. this had the side effect of not allowing hex values, but also is a more confusing experience.

Behavior After Change

  • Field lets you enter any text including letters and digits. If Number(val) returns NaN the value is considered invalid and the input will be red until corrected. This means you can enter hex values and after you finish editing they'll be converted to decimal. This is how it used to work before refactor: Use input type=number for field_number.ts #6845
  • On mobile with modal inputs enabled (default) there is no change in behavior - we use window.prompt so this change is irrelevant
  • On mobile (iOS and Android) with modal inputs disabled, you will get the standard keyboard and the same experience as desktop.

Reason for Changes

We need to allow Infinity

Test Coverage

Tested in browser stack on ios, android, and desktop

Documentation

Additional Information

@maribethb maribethb requested a review from a team as a code owner April 28, 2023 19:43
@maribethb maribethb requested a review from BeksOmega April 28, 2023 19:43
@github-actions github-actions bot added the PR: fix Fixes a bug label Apr 28, 2023
@github-actions github-actions bot added PR: fix Fixes a bug and removed PR: fix Fixes a bug labels May 3, 2023
@maribethb maribethb changed the title fix: use inputMode instead of type for number fields fix: remove input type from number field Jun 14, 2023
@github-actions github-actions bot added PR: fix Fixes a bug and removed PR: fix Fixes a bug labels Jun 14, 2023
@maribethb
Copy link
Contributor Author

@BeksOmega ready for re-review

@maribethb maribethb merged commit 438df87 into google:develop Jun 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: fix Fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants