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 TextField parse functionality #3824

Merged
merged 5 commits into from Sep 30, 2023

Conversation

manuelmeister
Copy link
Member

@manuelmeister manuelmeister commented Sep 20, 2023

I've corrected the way we handle numbers in our e-textfield and api-textfield. Currently this transformation is done at the wrong place. For v-model we can use the .number modifier to get a number. For the api-textfield I've implemented a parse function, that parses the value if attribute type="number" or inputmode="numeric" is set.

Currently in the MaterialTable it was not possible to have fraction numbers (1.2 kg Mehl). With this change it is now possible to input these values. On mobile the number keyboard is shown if you set inputmode="numeric".

@manuelmeister manuelmeister added the deploy! Creates a feature branch deployment for this PR label Sep 20, 2023
@manuelmeister manuelmeister temporarily deployed to feature-branch September 20, 2023 20:24 — with GitHub Actions Inactive
@github-actions
Copy link

github-actions bot commented Sep 20, 2023

Feature branch deployment currently inactive.

If the PR is still open, you can add the deploy! label to this PR to trigger a feature branch deployment.

@manuelmeister manuelmeister temporarily deployed to feature-branch September 23, 2023 16:17 — with GitHub Actions Inactive
@manuelmeister manuelmeister temporarily deployed to feature-branch September 23, 2023 18:00 — with GitHub Actions Inactive
@manuelmeister manuelmeister temporarily deployed to feature-branch September 23, 2023 18:19 — with GitHub Actions Inactive
@manuelmeister manuelmeister changed the title Bugfix/controls Fix TextField parse functionality Sep 23, 2023
@manuelmeister manuelmeister temporarily deployed to feature-branch September 23, 2023 18:25 — with GitHub Actions Inactive
@manuelmeister manuelmeister marked this pull request as ready for review September 23, 2023 18:30
@BacLuc
Copy link
Contributor

BacLuc commented Sep 24, 2023

I've corrected the way we handle numbers in our e-textfield and api-textfield. Currently this transformation is done at the wrong place. For v-model we can use the .number modifier to get a number. For the api-textfield I've implemented a parse function, that parses the value if attribute type="number" or inputmode="numeric" is set.

Currently in the MaterialTable it was not possible to have fraction numbers (1.2 kg Mehl). With this change it is now possible to input these values. On mobile the number keyboard is shown if you set inputmode="numeric".

I cannot reproduce your bug.
On dev it was no problem to use fraction numbers.
https://dev.ecamp3.ch/camps/6973c230d6b1/Harry-Potter-Lager/material
I used "." for the English way to create fraction numbers, and "," for the german way.

@manuelmeister
Copy link
Member Author

manuelmeister commented Sep 24, 2023

In Chrome on the Mac, I have to enter the numbers with a comma, which doesn't make sense because I have set the region to Switzerland, so it should use the dot to indicate the decimal point. Just the problem on the Mac is poor UX for Swiss users.

On mobile Safari on iOS it is currently impossible to enter decimal numbers, which is really bad.

The number input type should only be used for incremental numbers, especially when spinbutton incrementing and decrementing are helpful to user experience.
Source: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/number#using_number_inputs

I think it is better to let users type in a field and and then show a parse error, than to just not "allow" decimal numbers for some users. (Because a Swiss user would never expect a decimal to be formatted with a comma)

Maybe in the future we can provide German parsing (with a comma) but until then, I would suggest we focus on our Swiss userbase.

Copy link
Member

@usu usu left a comment

Choose a reason for hiding this comment

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

Desktop auto save doesn't work for me

image

frontend/src/components/form/base/ETextField.vue Outdated Show resolved Hide resolved
@manuelmeister manuelmeister temporarily deployed to feature-branch September 29, 2023 19:04 — with GitHub Actions Inactive
@manuelmeister manuelmeister temporarily deployed to feature-branch September 30, 2023 16:58 — with GitHub Actions Inactive
@manuelmeister
Copy link
Member Author

@usu can you test again? I could not reproduce your problem.

@usu
Copy link
Member

usu commented Sep 30, 2023

@usu can you test again? I could not reproduce your problem.

Seems to work now. Couldn't reproduce either

@manuelmeister manuelmeister added this pull request to the merge queue Sep 30, 2023
Merged via the queue into ecamp:devel with commit aee8a45 Sep 30, 2023
28 checks passed
@manuelmeister manuelmeister deleted the bugfix/controls branch September 30, 2023 23:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deploy! Creates a feature branch deployment for this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants