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

Taginput and cyrillic symbol "б" #1485

Closed
ValeriyMaslenikov opened this issue Jun 20, 2019 · 11 comments · Fixed by #1940
Closed

Taginput and cyrillic symbol "б" #1485

ValeriyMaslenikov opened this issue Jun 20, 2019 · 11 comments · Fixed by #1940

Comments

@ValeriyMaslenikov
Copy link

ValeriyMaslenikov commented Jun 20, 2019

Overview of the problem

Buefy version: 0.7.7
Vuejs version: 2.6.10
OS/Browser: Mac OS

Description

In the Cyrillic keyboard layout (for example in the Russian language) to type letter б you should press the same button that types , in the Latin layout (keycode: 188). After pressing this button in the taginput component, I expect to see the typed letter in the default behavior (without overriding confirmKeyCodes) but actually got confirmed tag.

In the Cyrillic layout, pressing Shift+6 causes typing a comma, but I would expect to see the confirmed tag.

As a solution, we can handle the keypress event in combination with keydown. keypress event has information about char code, where б and , have different codes.

Steps to reproduce

  1. Add simple component (without overriding confirmKeyCodes:
<b-taginput
  v-model="tags"
  ellipsis
  icon="label"
  placeholder="Add a tag">
</b-taginput>
  1. Type word which contains symbol "б", like "Абсолютно" and press enter.
  2. This word should create only one tag, but got two:

image

P.S. It's not duplication of #1148

@afwn90cj93201nixr2e1re
Copy link
Contributor

Большинство компонентов написано фиг пойми как и не тестируются меинтейнером, при этом багфиксы без объяснений - не принимаются. Привыкай.
https://github.com/buefy/buefy/blob/dev/src/components/taginput/Taginput.vue#L124
Свой массив передай.
И ниже тоже в separator, там где запятая, если нужно конечно.

@afwn90cj93201nixr2e1re
Copy link
Contributor

@afwn90cj93201nixr2e1re
Copy link
Contributor

@jtommy maybe you should merge it? I already tested.

@afwn90cj93201nixr2e1re
Copy link
Contributor

@service-paradis create pr with this fixes, coz @jtommy take some drugs and started do some stupid things again, xD.

@service-paradis
Copy link
Collaborator

service-paradis commented Aug 31, 2019

In fact, we would need to rewrite some component events that are using the deprecated KeyCode

The problem with KeyboardEvent.key right now is to support IE11 that uses an early version of the spec (for example, it uses "Esc" rather than "Escape"). Not a hard job, but would need to verify every key used to be sure to support IE (by using if (key === 'Escape' || key === 'Esc') instead of if (key === 'Escape') for example.

There is also an open issue because it does not always work when combining keys (https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/742263/). I also got an issue when using comma. IE11 is sometimes returning me "Decimal" instead of "," since the decimal separator for french canadian is a comma.

So, the commit you made is probably a good start but wont work for any cases. Fortunately, there is a great guide with all the keys and exceptions. Would need to make a lot of tests using different browsers to be sure to be compatible.

@afwn90cj93201nixr2e1re
Copy link
Contributor

afwn90cj93201nixr2e1re commented Aug 31, 2019

We can just create some array of keycodes and other stuff like that, and create some handler inside https://github.com/buefy/buefy/tree/dev/src/directives dir and that's all.
In this component we doesnt need in another separate key's, like esc and other stuff like that, esc gonna unfocus input with default beh.
So, my fix in this case is perfect now.
But in another cases(components), yep, we should make tests.

So, i can confirm that's not IE bug, this is normal behavior for Windows(testsed on some c++ stuff).

Also from link that you given, they wont fix it, coz seems like that's not bug.

@service-paradis
Copy link
Collaborator

@jtommy
We should think about switching to KeyboardEvent.key instead of the deprecated keyCode.

It would be a breaking change in some component where we can define key codes in prop (TagInput for example). So maybe we could label this issue as breaking change?

@jtommy
Copy link
Member

jtommy commented Oct 15, 2019

Sure!

@afwn90cj93201nixr2e1re
Copy link
Contributor

we should make more brekabke changes with fixes since 0.7.x in 0.9 release.

@sergpy
Copy link

sergpy commented Oct 28, 2019

maybe add a property to the component that blocks comma processing,
like as comma: bool
seems it the simplest way

@service-paradis
Copy link
Collaborator

In the next breaking release, we will cange the way Buefy handles Key events. It will solve this problem without the need to add a new prop.

service-paradis added a commit to service-paradis/buefy that referenced this issue Nov 5, 2019
service-paradis added a commit to service-paradis/buefy that referenced this issue Apr 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants