fix(input, input-number): allows numeric characters. by anveshmekala · Pull Request #7213 · Esri/calcite-design-system

anveshmekala · 2023-06-22T21:38:02Z

Related Issue: #6854

Summary

This PR will support French AZERTY keyboard layout in calcite-input & calcite-input-number

…ard-layout

jcfranco · 2023-06-23T20:12:40Z

    }
  });

+  // refer to issue here https://github.com/Esri/calcite-components/issues/6854


Can you add the issue number in the test name instead? Similar to https://github.com/Esri/calcite-components/blob/14036c1354419222bc5b839af8b68675b17ebf62/packages/calcite-components/src/components/tree/tree.e2e.ts#L962.

jcfranco · 2023-06-23T20:14:24Z

      return;
    }

+    // identifies French AZERTY keyboard input


Can you generalize this comment? I think this would apply to other keyboard layouts. Bonus points if you use a self-documenting const for this. ⭐

Actually, do we need to care about the Shift key being pressed? Wouldn't we want to process any number from KeyboardEvent.key? I tracked that part of logic down to #2127. @eriklharper Do you recall why we bail when the Shift key is pressed? This could possibly extend to the Alt, Ctrl and Meta keys.

The reason for verifying Shift key here is to make sure the numbers are being returned. In AZERTY keyboard , users need to keep shift key activated for numbers to be printed.

Ideally we would like to process any number but as you mentioned there is a custom logic that prevents numberKeys, ArrowKeys and Meta keys with Tab+Shift which results in the bug #6854. I wonder if there will be a case where the event.key will return a numberKey when Shift+Tab or Shift key is activated.

Either we need to remove numberKeys from supportedKeys array or remove the check for shiftKey. With input and input-number , i am very hesitant to remove any existing code since it is very impractical to iterate tests for every single locale we support.

The reason for verifying Shift key here is to make sure the numbers are being returned. In AZERTY keyboard , users need to keep shift key activated for numbers to be printed.

I wonder if there will be a case where the event.key will return a numberKey when Shift+Tab or Shift key is activated.

This is what I'm saying. In AZERTY, when the key combination for a number is pressed, you get a number in KeyboardEvent.key.

or remove the check for shiftKey.

Unless I'm missing something, I think this is the way to go (Shift + Tab being the exception).

jcfranco · 2023-06-23T20:16:23Z

    });

-    it("disallows typing any letter or number with shift modifier key down", async () => {
+    it("disallows typing any characters with shift modifier key down", async () => {


Would non-numeric characters be more precise for this?

Applies to other test names.

jcfranco

⌨️⌨️⌨️⌨️⌨️⌨️⌨️⌨️⌨️⌨️⌨️⌨️⌨️⌨️⌨️⌨️⌨️⌨️⌨️⌨️⌨️⌨️⌨️⌨️
⌨️🇫🇷🇫🇷🇫🇷🇫🇷⌨️⌨️🇫🇷🇫🇷⌨️⌨️⌨️🇫🇷🇫🇷⌨️⌨️🇫🇷⌨️⌨️⌨️🇫🇷⌨️🇫🇷⌨️
⌨️🇫🇷⌨️⌨️🇫🇷⌨️🇫🇷⌨️⌨️🇫🇷⌨️🇫🇷⌨️⌨️🇫🇷⌨️🇫🇷🇫🇷⌨️🇫🇷🇫🇷⌨️🇫🇷⌨️
⌨️🇫🇷🇫🇷🇫🇷⌨️⌨️🇫🇷⌨️⌨️🇫🇷⌨️🇫🇷⌨️⌨️🇫🇷⌨️🇫🇷⌨️🇫🇷⌨️🇫🇷⌨️🇫🇷⌨️
⌨️🇫🇷⌨️⌨️🇫🇷⌨️🇫🇷⌨️⌨️🇫🇷⌨️🇫🇷⌨️⌨️🇫🇷⌨️🇫🇷⌨️⌨️⌨️🇫🇷⌨️⌨️⌨️
⌨️🇫🇷🇫🇷🇫🇷🇫🇷⌨️⌨️🇫🇷🇫🇷⌨️⌨️⌨️🇫🇷🇫🇷⌨️⌨️🇫🇷⌨️⌨️⌨️🇫🇷⌨️🇫🇷⌨️
⌨️⌨️⌨️⌨️⌨️⌨️⌨️⌨️⌨️⌨️⌨️⌨️⌨️⌨️⌨️⌨️⌨️⌨️⌨️⌨️⌨️⌨️⌨️⌨️

jcfranco · 2023-06-24T01:24:32Z

@anveshmekala I think the PR title can be generalized since this now applies to other keyboard layouts besides AZERTY.

🤖 I have created a release *beep* *boop* --- <details><summary>@esri/calcite-components: 1.4.3</summary> ## [1.4.3](https://github.com/Esri/calcite-components/compare/@esri/calcite-components@1.4.2...@esri/calcite-components@1.4.3) (2023-06-26) ### Bug Fixes * **accordion-item:** support items working across shadowDOM ([#7035](#7035)) ([6378e35](6378e35)), closes [#6167](#6167) * **alert:** Sets autoCloseDuration to "medium" by default ([#7157](#7157)) ([1b9a8ed](1b9a8ed)) * **alert:** Update alert queue when an alert is removed from the DOM ([#7189](#7189)) ([edd59eb](edd59eb)) * **combobox, dropdown, input-date-picker, input-time-picker, popover, tooltip:** Prevent repositioning from affecting other floating components ([#7178](#7178)) ([1b02dae](1b02dae)) * Ensure mouse events are blocked for disabled components in Firefox ([#7107](#7107)) ([271d985](271d985)) * **input-date-picker:** Fix showing the placeholder when resetting the value ([#7156](#7156)) ([8d60ffd](8d60ffd)) * **input, input-number:** Allows numeric characters. ([#7213](#7213)) ([739f0af](739f0af)) * **input,input-number:** Allow typing decimal separator in firefox for arabic locale ([#7173](#7173)) ([595e6f2](595e6f2)) * **list:** No longer has incorrect border width ([a810943](a810943)) * **list:** Update selectedItems property on all item selection changes ([#7204](#7204)) ([da048f6](da048f6)) * **menu-item:** Ensure correct order of rendered icons ([#7098](#7098)) ([fd344e9](fd344e9)), closes [#7097](#7097) * **navigation:** Label is no longer a required property ([#7084](#7084)) ([ba2bd4d](ba2bd4d)) * **radio-button-group:** No longer focus first radio button on label click and adds `setFocus` method. ([#7050](#7050)) ([4267b8c](4267b8c)) * **radio-button, radio-button-group:** Prevent emitting events when selecting a checked radio button ([#7102](#7102)) ([77fcc81](77fcc81)) * **radio-button:** Focuses first focusable radio-button element in group. ([#7152](#7152)) ([dd7ec60](dd7ec60)) * **scrim:** Responsively set the scale of the loading spinner ([#7182](#7182)) ([72c5943](72c5943)) * **tooltip:** Improve component timing ([#7172](#7172)) ([106f5d2](106f5d2)) * **tree-item:** Ensure expanded tree-item is displayed when expanded and made visible ([#7216](#7216)) ([3c0fbf5](3c0fbf5)) <details><summary>@esri/calcite-components-react: 1.4.3</summary> ## [1.4.3](https://github.com/Esri/calcite-components/compare/@esri/calcite-components-react@1.4.2...@esri/calcite-components-react@1.4.3) (2023-06-26) ### Miscellaneous Chores * **@esri/calcite-components-react:** Synchronize undefined versions ### Dependencies * The following workspace dependencies were updated * dependencies * @esri/calcite-components bumped from 1.4.3-next.7 to 1.4.3 </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Ben Elan <no-reply@benelan.dev>

…issue-template-checkbox * origin/master: (32 commits) ci: double build cc when publishing to workaround stencil types bug (#7227) build: bump package versions back to 1.5.0-next.5 (#7228) chore: release latest (#7144) ci: hardcode release version due to reverting feat (#7225) fix(tree-item): ensure expanded tree-item is displayed when expanded and made visible (#7216) ci(release-please): pin action version and allow manually running action (#7222) fix(input, input-number): allows numeric characters. (#7213) build(t9n): generate json file containing t9n values (#7214) chore: release next fix(combobox, dropdown, input-date-picker, input-time-picker, popover, tooltip): Prevent repositioning from affecting other floating components (#7178) build: update browserslist db (#7192) build: ignore node_modules and build outputs when watching for changes during stencil tests (#7209) test: set up `disabled` helper to run a test per use case (#7089) ci: set design complete label conditionals (#7206) chore: release next fix(list): update selectedItems property on all item selection changes (#7204) chore: fix sorting logic for t9nmanifest entries (#7203) fix(radio-button): focuses first focusable radio-button element in group. (#7152) chore: release next fix(alert): update alert queue when an alert is removed from the DOM (#7189) ...

fix(input,input-number): supports French AZERTY keyboard layout

b4cc71c

github-actions Bot assigned anveshmekala Jun 22, 2023

github-actions Bot added the bug Bug reports for broken functionality. Issues should include a reproduction of the bug. label Jun 22, 2023

anveshmekala added 3 commits June 22, 2023 17:41

fix tests

fbc5210

remove unused imports

0eb2956

Merge branch 'master' into anveshmekala/6854-input-allow-AZERTY-keybo…

5cb15ec

…ard-layout

anveshmekala marked this pull request as ready for review June 22, 2023 23:25

anveshmekala requested a review from a team as a code owner June 22, 2023 23:25

remove extra space in test header

ec55735

anveshmekala added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Jun 22, 2023

anveshmekala marked this pull request as draft June 22, 2023 23:52

add tests

d51b3ad

anveshmekala marked this pull request as ready for review June 23, 2023 16:01

anveshmekala added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Jun 23, 2023

jcfranco reviewed Jun 23, 2023

View reviewed changes

anveshmekala added 2 commits June 23, 2023 18:48

feedback changes

f6ef851

remove newline spaces

8136b4a

jcfranco approved these changes Jun 23, 2023

View reviewed changes

jcfranco changed the title ~~fix(input,input-number): supports French AZERTY keyboard layout~~ fix(input, input-number): supports French AZERTY keyboard layout Jun 23, 2023

anveshmekala added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Jun 25, 2023

anveshmekala changed the title ~~fix(input, input-number): supports French AZERTY keyboard layout~~ fix(input, input-number): allows numeric characters. Jun 25, 2023

anveshmekala merged commit 739f0af into master Jun 26, 2023

anveshmekala deleted the anveshmekala/6854-input-allow-AZERTY-keyboard-layout branch June 26, 2023 15:21

github-actions Bot added this to the 2023 June patch priorities milestone Jun 26, 2023

calcite-admin mentioned this pull request Jun 26, 2023

chore: release latest #7144

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(input, input-number): allows numeric characters.#7213

fix(input, input-number): allows numeric characters.#7213
anveshmekala merged 8 commits intomasterfrom
anveshmekala/6854-input-allow-AZERTY-keyboard-layout

anveshmekala commented Jun 22, 2023 •

edited

Loading

Uh oh!

jcfranco Jun 23, 2023

Uh oh!

jcfranco Jun 23, 2023

Uh oh!

anveshmekala Jun 23, 2023

Uh oh!

jcfranco Jun 23, 2023

Uh oh!

jcfranco Jun 23, 2023

Uh oh!

jcfranco left a comment

Uh oh!

jcfranco commented Jun 24, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

anveshmekala commented Jun 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

jcfranco Jun 23, 2023

Choose a reason for hiding this comment

Uh oh!

jcfranco Jun 23, 2023

Choose a reason for hiding this comment

Uh oh!

anveshmekala Jun 23, 2023

Choose a reason for hiding this comment

Uh oh!

jcfranco Jun 23, 2023

Choose a reason for hiding this comment

Uh oh!

jcfranco Jun 23, 2023

Choose a reason for hiding this comment

Uh oh!

jcfranco left a comment

Choose a reason for hiding this comment

Uh oh!

jcfranco commented Jun 24, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

anveshmekala commented Jun 22, 2023 •

edited

Loading