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

NumericField: fix a number of issues, i.e. typing numbers in French and other locales #2643

Merged
merged 4 commits into from
Sep 7, 2021

Conversation

mckaragoz
Copy link
Member

@mckaragoz mckaragoz commented Sep 5, 2021

We handled and should be solved (especially on server-side) several issues: #2146, #2487, #2583, #2609, #2632; and it complete and works well with #2626.

  1. MudNumericField now didn't accept letter keys on pressed (before it allows, but removes when lost focus)
  2. Added AZERTY support.
  3. Before when we press up or down button on Immediate="False" numeric field, it increase or decrease only once, and then it lost focus and pressing key will scroll the page, now it seems to accept continuously.
  4. Before when we press up or down button on Immediate="True" numeric field, it increases or decreases, but it didn't see the last value until lost focus, now it both accept and show value.
  5. Fix Enter and NumpadEnter buttons that they can eligible for users' own keypress events.

UPDATE:
6. Added support for Select All, Copy and Paste (Ctrl+a, Ctrl+c, Ctrl+v)

Before:

20210905_010428.mp4

After:

20210905_010742.mp4

fixes #2146, fixes #2487, fixes #2583, fixes #2609, fixes #2632

@henon
Copy link
Collaborator

henon commented Sep 5, 2021

@JohanArleth would you please be so kind as to check if the NumericField still behaves correctly with respect to the things we worked on in our last collab #2136?

@henon
Copy link
Collaborator

henon commented Sep 5, 2021

@mckaragoz did you test on both BSS and WASM?

@mckaragoz
Copy link
Member Author

@henon i added WASM support. WASM have the 3rd and 4th problems as BSS have before, and I noticed we can do same process with BSS and WASM to solve this.

Only problem after this changes, when we press arrow keys, input value changed, but page is also scrolled. To solve this i added an extra preventDefault things which are have differencies between BSS and WASM and explain it comments in code.

I noticed that MudNumericField didn't accept the Ctrl+a, c, v (select all, copy, and paste) progress with keyboard, and i added support also for this.

After this all, the only little thing is, when we hold down the arrow key, page may also scroll. But user always can prevent this by pressing Shift+Arrow, so i think this is not a big problem considering current gains.

@henon
Copy link
Collaborator

henon commented Sep 5, 2021

Awesome @mckaragoz. I would like for @JohanArleth to look over it just in case, so we don't have any accidental regressions.

@henon henon changed the title MudNumericField Major Changes NumericField: fix a number of issues Sep 5, 2021
@henon henon changed the title NumericField: fix a number of issues NumericField: fix a number of issues, i.e. typing numbers in French and other locales Sep 5, 2021
@JohanArleth
Copy link
Contributor

Using the test viewer, I'm getting exceptions in the console on every arrow up/down press. It still seems to work correctly though. (remove Position="Position.Absolute" from MudAppBar to get the test viewer working. Maybe push this fix in the pr as well?)

There is inconsistent behavior when deleting the value in the field (both immediate and not). Using a nullable backing property, as in the numeric culture test component, it will first default to 0.00 when clearing the field, and on second attempt the value is still 0.00, but the field becomes empty. I'm not sure if this is a new or old behavior.

https://i.imgur.com/U3GbTtS.gif

Error on arrow
https://i.imgur.com/2Qpt8av.png

If you feel like it, it would be great to also get support for "Clearable" on numeric fields.

@mckaragoz
Copy link
Member Author

mckaragoz commented Sep 6, 2021

Using the test viewer, I'm getting exceptions in the console on every arrow up/down press.

I have found an issue maybe the exception related to this dotnet/aspnetcore#26838. This exception may be occurring due to onfocusout. They moved this issue a in .Net 6 preview 4 but i don't know when they implement a solution. I think this isn't directly about us. And we may ignore this for now.

remove Position="Position.Absolute" from MudAppBar to get the test viewer working. Maybe push this fix in the pr as well?

@henon i can handle it if the team want.

I'm not sure if this is a new or old behavior.

I tried this another branch which doesn't have the commit, but the behaviour is the same in test. I don't understand the reason of the behaviour, maybe we should add a new issue after implement this. I add clear button to the test which set value to null on click, and it works properly, so it may about the backspace key and pressing behaviour.

If you feel like it, it would be great to also get support for "Clearable" on numeric fields.

Clearable icon is overlapping the HideSpinButton (it only works when we set hidespin to true), and im not good at CSS :) we need to change their position to opposite of other or when we opened one, the other automatically hide.

@henon
Copy link
Collaborator

henon commented Sep 7, 2021

OK, sure, you can PR the testviewer fix as well. Thanks.
I will merge this, as it seems to work fine. Clearable support can be done in a new PR

@henon henon merged commit 521a097 into MudBlazor:dev Sep 7, 2021
@henon
Copy link
Collaborator

henon commented Sep 7, 2021

Big thanks to @mckaragoz ! You saved me a lot of headache by fixing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants