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

New features, improvements, & bug fixes for text/number input #12891

Conversation

AustinPhillipTaylor
Copy link
Contributor

New options for text input interface:

  • Field prefix/suffix
  • Enable/disable spellcheck (Default to off)
  • Enable/disable displaying number arrows (Default to show)

Screen Shot 2022-04-20 at 1 38 25 PM

Screen Shot 2022-04-20 at 1 38 42 PM

New options for number input interface:

  • Field prefix/suffix
  • Enable/disable displaying number arrows (Default to show)
  • Enable/disable displaying data type checking warnings (Default to show)

Screen Shot 2022-04-20 at 1 43 03 PM

Bug fixes:

  • Previously, number inputs would cast everything to a javascript number. This limited the precision of everything to that of a 64-bit float. As a result, the Big Integer and Decimal data types were completely useless unless the user was only setting them via a custom API call.
    • These changes implement the Big.js NPM package for handling numbers of variable size and precision.

New features/ Improvements:

  • Affix can be set from interface options
  • Numbers in text fields can be incremented and decremented
    • In steps of 1 (only handles positive, whole numbers)
    • Based on cursor position
    • Using arrow keys or optional arrow buttons
  • Number fields run checks to loosely determine if content conforms to data type constraints
    • Warnings can be disabled
  • By default incrementing and decrementing number fields will move up or down by the step. Holding shift when pressing/clicking the arrows will snap the value to exact multiples of the step. For instance, if the step is set to 5, and the value is currently 12, pressing the up arrow will change the value to 17. However, shift + up arrow will change the 12 to 15.
    • This differs from the built-in number input where it enforces the value always being an exact multiple of step.
Screen.Recording.2022-04-20.at.2.03.07.PM.mp4

@AustinPhillipTaylor
Copy link
Contributor Author

I've tested this quite a bit, however, an extensive stress-test would probably be a good idea since the v-input component is so integral to other interfaces.

@AustinPhillipTaylor
Copy link
Contributor Author

I believe this should also peripherally resolve #5697

@benhaynes
Copy link
Sponsor Member

First off, this is cool stuff! I wouldn't have even thought of some of these, but they are fun to see! We'll have to check the 80/20 and make sure the code is worth being in Core (as opposed to an extension), but I think some of this makes sense in Core (number features in an INT interface). The string/number combos get a little more complex... if we support arrow in inputs, then it might be expected in textareas... but then UP/DOWN would be navigational. Food for thought.

The only other thing I wondered about was the prefix/suffix. I love it, but there's overlap with a feature I'd really like to see, which is properly masked fields. If we get masked inputs, we could also do 123-456-7890 (phone), or other, more complex, use-cases. We don't want overlapping features/code, and it might be worth looking at masking, since that's so much more powerful and also satisfies prefix/suffix.

My only last point is about options. I don't think what you have here is way over the top, but we should be careful about adding too much flexibility into the options, and then making setup more complex/confusing. "Advanced Options" and smart defaults could partly solve this, but just pointing it out. :)

All in all, great work! For big PRs like this, let's make sure we coordinate with the Core Team as much as possible beforehand so we're all aligned. I want to make sure all dev time is heading in the right direction. ❤️

@AustinPhillipTaylor
Copy link
Contributor Author

AustinPhillipTaylor commented Apr 21, 2022

@benhaynes Absolutely understand what you're saying with the up/down possibly being expected on textareas as well. Since I ultimately limited that functionality to only affecting whole numbers, it actually wouldn't be difficult to implement on textareas. The regex is a pretty inexpensive operation, just looking for digits. With regard to the arrows being used as navigation in a textarea, that could be easily maintained by requiring an extra system key to activate the number incrementation. For instance, maybe the user has to hold option/alt when they press the arrow key, to modify numbers. But that's a discussion for another day.

The prefix/suffix is purely presentational, so it shouldn't affect masked inputs. In fact, they're already part of the current v-input component (Something I didn't realize before I started working on this). All I did there was expose those props to the interface so users could add an affix (currently you can only add them if you directly use the v-input component, for instance, when building a module). I had considered making the affixes part of the value of the component, but decided against it as I realized it wouldn't fit the 80/20 rule, and it would probably have caused issues down the line.

In terms of the options, I agree that having a simple, quick-setup flow is important. On the text input, all of the new options are hidden under the advanced section. The standard options are the same as they were before. However, that was an oversight I made on the number input options. Currently, all of the new options are part of the standard setup options. I'm going to go ahead and make a new commit right now to hide some of those settings under the advanced options. That one is my bad 😅

Other than those points, I think all of the number parsing changes on the number input are definitely necessary. I'm genuinely surprised that no one had reported before that it didn't properly handle Big Integer, or Decimal data types. As soon as I noticed that, I just had to fix it. On that note, I do apologize for not coordinating more beforehand. I got into the flow and didn't realize how much I had done until it was finished. I'll be more mindful from now on 👍

@bamboowonder
Copy link

a feature I'd really like to see, which is properly masked fields. If we get masked inputs, we could also do 123-456-7890 (phone), or other, more complex, use-cases.

Big +1 on masked fields so we can more easily ensure users are providing valid input to the database. Phone numbers, Postal Codes all benefit big from this.

@KAVA-Leigh
Copy link

a feature I'd really like to see, which is properly masked fields. If we get masked inputs, we could also do 123-456-7890 (phone), or other, more complex, use-cases.

Big +1 on masked fields so we can more easily ensure users are providing valid input to the database. Phone numbers, Postal Codes all benefit big from this.

Another +1 on this, masking would be really helpful :)

@rijkvanzanten
Copy link
Member

Heya, thanks for your PR! Our small team has been attempting to work through a backlog of external contributions for the better part of a year, but in that time many have become too stale to merge or the change is no longer desired/relevant..
We've updated the contributing guidelines and are working on making more resources available to make sure that doesn’t happen again 😬 In the meantime, we'll have to close this PR for now..

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants