Skip to content

ERA-10520: Support EFB schemas in ER > Numeric field #1208

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

Merged
merged 33 commits into from
Jan 23, 2025
Merged

Conversation

gaboDev
Copy link
Contributor

@gaboDev gaboDev commented Dec 12, 2024

What does this PR do?

  • It adds EFB support for numeric fields

How does it look

  • image

Relevant link(s)

Where / how to start reviewing (optional)

  • I added an mocked schema with several configurations for numeric fields

@gaboDev
Copy link
Contributor Author

gaboDev commented Dec 12, 2024

Regarding the disadvantages of the native numeric input type, I have replaced it with a simple text with some tweaks which will improve the UX and the accessibility for the input, anyways, event if this text input is behaving as a numeric one it doesn't render the control of the counter (the two arrows for augmenting/reducing the input value)

I'm more the happy to add that missing piece, just wanted to ask about your opinion if that would be a worthy effort 🫡

cc
@luixlive @JoshuaVulcan

@gaboDev
Copy link
Contributor Author

gaboDev commented Dec 12, 2024

Regarding the disadvantages of the native numeric input type, I have replaced it with a simple text with some tweaks which will improve the UX and the accessibility for the input, anyways, event if this text input is behaving as a numeric one it doesn't render the control of the counter (the two arrows for augmenting/reducing the input value)

I'm more the happy to add that missing piece, just wanted to ask about your opinion if that would be a worthy effort 🫡

cc @luixlive @JoshuaVulcan

As discussed with the team, I will be creating a re-usable custom component for input numeric, this will give us the opportunity to use it not only in the EFB support but in the entire application when required

@gaboDev
Copy link
Contributor Author

gaboDev commented Jan 21, 2025

few questions for the team after discussing implementation with @gaboDev :

  1. the current implementation throws a warning/error when the input is outside the given range in the schema until the user hits save.

With this new implementation, users are not allowed to type numbers outside of the min/max range, which prevents them from reaching that warning message. I feel that we should be consistent between the two implementations, allowing users to type values and throwing the message upon saving changes.

  1. scroll-down is active on previous implementation, for consistency i had the feeling that we should allow that here too, however, @gaboDev pointed out that there is a popular concern on that behavior, bc it might add undesired changes on input if they mistakenly scroll. i agreed w/ him, but just wanted to make sure everyone is ok w/ that.

3.- what should be the behavior when there are float/integer equivalences? if a user types 2.0, should we turn it into 2? or should we save the value as it was entered? my suggestion is that we keep the value as it was entered, and not turn it into its integer counterpart.

cc: @luixlive @JoshuaVulcan @tiffanynwong

I've added a prop to the custom component that controls numeric range validation, so for the Numeric EFB implementation I have deactivated this feature, meaning a user can type numbers out of the range (also applies for the arrow buttons) letting the validation happens only at submit (also added a more meaningful validation error)

Regarding the point 3, I have added a piece of logic to keep the 2.0 or 5.0 or any X.0 number shown with the digits, anyway the data is being stored as plain integer number, this due the parsing mechanism of JS.

Meaning the user is allowed to use/type X.0 numbers but once the event is saved the number will be stored as a plain integer.

@AlanCalvillo

@luixlive
Copy link
Contributor

Just providing my share:

  1. the current implementation throws a warning/error when the input is outside the given range in the schema until the user hits save.

Yes, and with the new forms we don't want those native validations anymore. They are inconsistent in different browsers and disruptive with the flow of a HTML form. We are now handling our own error messages on submission and our fields are more restrictive so the users don't reach an error state (we try to enforce valid values while the user fills the inputs).

  1. scroll-down is active on previous implementation [...]

Yes, and just as you mentioned, it's a concern around web technologies. Most libraries disable that functionality since it's pretty common that users scrolling a form modify the value in the input without noticing.

I feel like we shouldn't really try to stay with the old implementation if what we are doing enhancements to the fields. Adding our custom error messages, following best practices or adding mechanisms to help the user navigate or fill the form faster may make the experience a bit different and will be new to users, but should be better in the short term @AlanCalvillo .

@AlanCalvillo
Copy link
Contributor

Yes, and with the new forms we don't want those native validations anymore. They are inconsistent in different browsers and disruptive with the flow of a HTML form. We are now handling our own error messages on submission and our fields are more restrictive so the users don't reach an error state (we try to enforce valid values while the user fills the inputs).

That context about inconsistency makes sense. I'm just doubtful about the possible confusion it might create for users who consume a form(but didn't create it themselves) and when trying to input a value outside the range, then the UI will not allow them to do so, and won't have any visual feedback. Hints/Description should add clarity, though.
I guess we can go ahead with the implementation of this PR, and try to gather feedback from users on how much confusion it creates if any.

@tiffanynwong
Copy link
Collaborator

Love the inline errors 😍

The way the input range is working now is great. @AlanCalvillo I'm seeing now that you can type in any numeric input and the field gets validated on save or if the user hits enter while the field is focused.

Some other feedback

  1. Typing invalid inputs: I get that since it's a numeric field text or certain symbols aren't acceptable but we should have some feedback to the user about what inputs are accepted in the field. That way the user isn't angrily typing in text and not seeing a response from the ui. Could we do something like this, or is this out of scope?
  • User types invalid inputs (letters, symbols)
  • the field changes to error state
  • the error message can say: Invalid input: Please enter numbers only.
  1. Out of range error message: Just an update to the text here to be more accurate. "This value should be greater than or equal to 10"
    Screen Shot 2025-01-22 at 10 05 42 AM
    And "This value should be less than or equal to 20"
    Screen Shot 2025-01-22 at 10 40 08 AM

  2. Meaning the user is allowed to use/type X.0 numbers but once the event is saved the number will be stored as a plain integer.

I agree w/ Alan here, we should keep whatever value the user types in. Since don't have a way for the user to specify formatting of the number inputed in the form builder we shouldn't assume that the input should be a whole number. It's very possible that the user will have to type in a value like 20.45 to say how much something cost for example.

@luixlive
Copy link
Contributor

@tiffanynwong

  • User types invalid inputs (letters, symbols)
  • the field changes to error state
  • the error message can say: Invalid input: Please enter numbers only.

I'd say this is getting a bit out of scope 😅 We haven't done anything similar to other fields. All validations are done on submission. Also, I'm not sure I've seen a form anywhere else with that kind of validations. Even native HTML numeric input simply won't allow you to enter invalid characters: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/number That's pretty standard. I feel like admins have some responsibility too to write good labels, descriptions and hints to help their users to understand the forms.

It's very possible that the user will have to type in a value like 20.45 to say how much something cost for example.

I think Gabo mentioned that the only values that are stored as integers are if they have only zeroes after the dot, like 20.0 -> 20. I don't think that is too bad, but anyway it may make sense to keep the format users want for their values.

@gaboDev
Copy link
Contributor Author

gaboDev commented Jan 22, 2025

I agree w/ Alan here, we should keep whatever value the user types in. Since don't have a way for the user to specify formatting of the number inputed in the form builder we shouldn't assume that the input should be a whole number. It's very possible that the user will have to type in a value like 20.45 to say how much something cost for example.

As Luis mentioned, currently only the values which has only a zero as decimal digit 3.0, 145.0 or 3.5 are stored as plain integer the reason is javascript data typing, but if the user enters a specific decimal number like 3.45 it will be stored as it is, a regular decimal number

and thanks for the feedback of the validation messages I will be addressing it
@tiffanynwong

@AlanCalvillo
Copy link
Contributor

Love the inline errors 😍

The way the input range is working now is great. @AlanCalvillo I'm seeing now that you can type in any numeric input and the field gets validated on save or if the user hits enter while the field is focused.

Yep, it got addressed after the comment 😄

@AlanCalvillo
Copy link
Contributor

the field behaves oddly when typing 0 in decimal values. for e.g: i can't type 1.09, but i can enter 109. if I paste the value 1.09 in the field being copied from elsewhere, it can be entered w/o issues.

I have added some logic to handle this scenario, it was already there just needed some tweaks

@AlanCalvillo

I think it's almost there. I'm just having issues when handling multiple zeroes:

  • only one zero can be added. When I try to type in 15.00, the second zero deletes the first for some reason.
  • also if I want to type, 15.908, the zero after the 9 can't be typed in. it can be pasted in, though.
Screen.Recording.2025-01-22.at.17.06.07.mov

@gaboDev
Copy link
Contributor Author

gaboDev commented Jan 23, 2025

the field behaves oddly when typing 0 in decimal values. for e.g: i can't type 1.09, but i can enter 109. if I paste the value 1.09 in the field being copied from elsewhere, it can be entered w/o issues.

I have added some logic to handle this scenario, it was already there just needed some tweaks
@AlanCalvillo

I think it's almost there. I'm just having issues when handling multiple zeroes:

  • only one zero can be added. When I try to type in 15.00, the second zero deletes the first for some reason.
  • also if I want to type, 15.908, the zero after the 9 can't be typed in. it can be pasted in, though.

Screen.Recording.2025-01-22.at.17.06.07.mov

Good catch @AlanCalvillo
I just sent a commit with the fixes 🫡

Copy link
Contributor

@AlanCalvillo AlanCalvillo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's friggin' gooo 💥 🚀 🚀 🚀

@gaboDev gaboDev merged commit b4727fd into develop Jan 23, 2025
3 checks passed
@gaboDev gaboDev deleted the ERA-10520 branch January 23, 2025 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants