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

Numpad comma separator support for Chips #2349

Closed
Christoph-Wagner opened this issue Mar 24, 2022 · 1 comment
Closed

Numpad comma separator support for Chips #2349

Christoph-Wagner opened this issue Mar 24, 2022 · 1 comment
Assignees
Labels
Type: Enhancement Issue contains an enhancement related to a specific component. Additional functionality has been add
Milestone

Comments

@Christoph-Wagner
Copy link
Contributor

Christoph-Wagner commented Mar 24, 2022

I'm submitting a ... (check one with "x")

[X] bug report => Search github for a similar issue or PR before submitting
[ ] feature request => Please check if request is not on the roadmap already https://github.com/primefaces/primevue/wiki/Roadmap
[ ] support request => Please do not submit support request here, instead see http://forum.primefaces.org/viewforum.php?f=110

Currently, the Chips-Component has the following code:

this.separator === ',' && event.which === 188

Besides which being deprecated, it also reports 110 for the numpad comma on German (and presumably others with , as decimal separator) keyboard layouts. event.key reports ,. event.code reports NumpadDecimal which is not helpful either. My suggestion would be to use event.key to check instead. Browser compatibility seems to be adequate. If wanted, I can create a PR for this.

CodeSandbox Case (Bug Reports)
The bug is basic and already affects the demo https://primefaces.org/primevue/#/chips

Current behavior
, does not work as separator when using the numpad

Expected behavior
, works as separator as documented, or the documentation needs a warning.

Minimal reproduction of the problem with instructions
Change the keyboard to German (decimal separator is , in German), try to use the , from the numpad (same position as . in the US Layout as separator.

What is the motivation / use case for changing the behavior?
It’s not working as documented

Please tell us about your environment:

Any OS using a German KB Layout

  • Vue version: 3.X
    Latest

  • PrimeVue version: 3.4.X
    Latest

  • Browser: Firefox, Chrome

@Christoph-Wagner Christoph-Wagner changed the title Comma-separator for Chips doesn’t work on all keyboard layouts Comma-separator for Chips doesn’t work on keyboard layouts with comma as decimal separator when using the numpad Mar 24, 2022
@tugcekucukoglu tugcekucukoglu added the Type: Bug Issue contains a bug related to a specific component. Something about the component is not working label Mar 28, 2022
@tugcekucukoglu tugcekucukoglu changed the title Comma-separator for Chips doesn’t work on keyboard layouts with comma as decimal separator when using the numpad Numpad comma separator support for Chips Mar 28, 2022
@tugcekucukoglu tugcekucukoglu added Type: Enhancement Issue contains an enhancement related to a specific component. Additional functionality has been add and removed Type: Bug Issue contains a bug related to a specific component. Something about the component is not working labels Mar 28, 2022
tugcekucukoglu added a commit that referenced this issue Mar 28, 2022
Fixed #2349 - Numpad comma separator support for Chips
@tugcekucukoglu tugcekucukoglu added this to the 3.12.3 milestone Mar 28, 2022
@tugcekucukoglu tugcekucukoglu self-assigned this Mar 28, 2022
@Christoph-Wagner
Copy link
Contributor Author

While that fixes the issue for German keyboards, it’s now broken for others. event.which is 110 for all Numpad DecimalSeparators, including those like en-us that use .. So with this change, it would be broken exactly for those layouts that did not have an issue before.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement Issue contains an enhancement related to a specific component. Additional functionality has been add
Projects
None yet
Development

No branches or pull requests

2 participants