Skip to content

Conversation

@zaquille-oneil
Copy link
Contributor

@zaquille-oneil zaquille-oneil commented Jul 20, 2023

WHY are these changes introduced?

Fixes an issue where TextFields that have the prop type="integer" do not properly handle onKeyDown events for ArrowUp and ArrowDown. Ideally, we increment these values by 1 multiple of the step value.

Context: While working on the Admin -> Products -> Catalogs surface, we have a need for the TextField component to be able to handle integer inputs while also providing the ability to increment upward and downward with keypresses of ArrowUp and ArrowDown.

Demo after the fix is applied:
https://screenshot.click/20-27-irbo7-6glak.mp4

WHAT is this pull request doing?

Resolves #9779
The integer input type is not a standard input type in HTML5. HTML5 does not provide a specific input type specifically for integers.

the web also doesn’t think that HTML5 inputs should take integer. if that’s the case, then it defaults to "text" . However, polaris textFields allow for an “integer” type when they create input elements. this pr has some context on why “integer” was introduced. here is some TextFields.tsx code that i pseudo-coded to shorten:

type Type =
  | 'number'
  | 'integer'
  | ...
...
interface TextFieldProps {
  inputType?: Type;
  ...
}
...
const input = createElement('input', {
  type: inputType,
  ...
}
...

Since <TextField type="integer" /> is essentially <input type="text" />, TextField.tsx will need to be responsible for handling some of the behavior that comes for free with "number" inputs. Some logic already exists but I noticed that "ArrowUp" and "ArrowDown" are specifically not being captured.

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

Copy-paste this code in playground/Playground.tsx:
import React from 'react';
import {Page} from '../src';

export function Playground() {
  return (
    <Page title="Playground">
      {/* Add the code you want to test in here */}
    </Page>
  );
}

🎩 checklist

@zaquille-oneil zaquille-oneil changed the title Added logic to capture ArrowUp and ArrowDown for 'integer' type TextF… [TextField] Improve "integer" type TextField's onKeyDown capture logic for arrow keys Jul 20, 2023
@stefanlegg
Copy link
Contributor

[nit] Dont forget to add a changeset for this - here's an example from one of my prs

@zaquille-oneil zaquille-oneil changed the title [TextField] Improve "integer" type TextField's onKeyDown capture logic for arrow keys [TextField] Improve TextField's onKeyDown capture logic for arrow keys Jul 20, 2023
@zaquille-oneil zaquille-oneil changed the title [TextField] Improve TextField's onKeyDown capture logic for arrow keys [TextField] Improve/Fix TextField's onKeyDown capture logic for arrow keys Jul 20, 2023
@zaquille-oneil zaquille-oneil marked this pull request as ready for review July 20, 2023 17:23
Copy link
Contributor

@stefanlegg stefanlegg left a comment

Choose a reason for hiding this comment

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

🎩 & code LGTM

Copy link
Member

@sam-b-rose sam-b-rose left a comment

Choose a reason for hiding this comment

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

@zaquille-oneil zaquille-oneil merged commit 571acc1 into main Jul 20, 2023
@zaquille-oneil zaquille-oneil deleted the ZL/text-field-integer-type-keydown-upgrade branch July 20, 2023 21:07
mattkubej pushed a commit that referenced this pull request Jul 21, 2023
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## @shopify/polaris-icons@7.6.0

### Minor Changes

- [#9777](#9777)
[`8228de0f6`](8228de0)
Thanks [@heyjoethomas](https://github.com/heyjoethomas)! - Added social
media icons

## @shopify/polaris@11.9.0

### Minor Changes

- [#9761](#9761)
[`ce3e516a2`](ce3e516)
Thanks [@aveline](https://github.com/aveline)! - Added `readOnly` prop
to `Labelled` component


- [#9770](#9770)
[`571acc166`](571acc1)
Thanks [@zaquille-oneil](https://github.com/zaquille-oneil)! - Updating
TextField to support ArrowUp and ArrowDown keypresses for "integer" type

### Patch Changes

- [#9765](#9765)
[`541e5920b`](541e592)
Thanks [@mattkubej](https://github.com/mattkubej)! - [TopBar] convert to
grid and center align search field

- Updated dependencies
\[[`8228de0f6`](8228de0)]:
    -   @shopify/polaris-icons@7.6.0

## @shopify/polaris-cli@0.2.24



## polaris.shopify.com@0.56.2

### Patch Changes

- Updated dependencies
\[[`ce3e516a2`](ce3e516),
[`8228de0f6`](8228de0),
[`571acc166`](571acc1),
[`541e5920b`](541e592)]:
    -   @shopify/polaris@11.9.0
    -   @shopify/polaris-icons@7.6.0

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
juzser pushed a commit to juzser/polaris that referenced this pull request Jul 27, 2023
… keys (Shopify#9770)

### WHY are these changes introduced?

Fixes an issue where TextFields that have the prop `type="integer"` do
not properly handle `onKeyDown` events for `ArrowUp` and `ArrowDown`.
Ideally, we increment these values by 1 multiple of the `step` value.

Context: While working on the **Admin -> Products -> Catalogs** surface,
we have a need for the `TextField` component to be able to handle
`integer` inputs while also providing the ability to increment upward
and downward with keypresses of `ArrowUp` and `ArrowDown`.

Demo after the fix is applied:
https://screenshot.click/20-27-irbo7-6glak.mp4
### WHAT is this pull request doing?
Resolves Shopify#9779
The integer input type is not a standard input type in HTML5. HTML5 does
not provide a specific input type specifically for integers.

[the
web](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input)
also doesn’t think that HTML5 inputs should take integer. if that’s the
case, then it defaults to "text" . However, polaris textFields allow for
an “integer” type when they [create input
elements](https://github.com/Shopify/polaris/blob/d0477410d90946916e9dadcaf5cef2461be1909d/polaris-react/src/components/TextField/TextField.tsx#L540C1-L540C1).
[this pr](Shopify#9051) has some context
on why “integer” was introduced. here is some TextFields.tsx code that i
pseudo-coded to shorten:
```
type Type =
  | 'number'
  | 'integer'
  | ...
...
interface TextFieldProps {
  inputType?: Type;
  ...
}
...
const input = createElement('input', {
  type: inputType,
  ...
}
...
```

Since `<TextField type="integer" />` is essentially `<input type="text"
/>`, TextField.tsx will need to be responsible for handling some of the
behavior that comes for free with `"number"` inputs. Some logic already
exists but I noticed that "ArrowUp" and "ArrowDown" are specifically not
being captured.

<!-- ℹ️ Delete the following for small / trivial changes -->

### How to 🎩

🖥 [Local development
instructions](https://github.com/Shopify/polaris/blob/main/README.md#local-development)
🗒 [General tophatting
guidelines](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md)
📄 [Changelog
guidelines](https://github.com/Shopify/polaris/blob/main/.github/CONTRIBUTING.md#changelog)

<!--
  Give as much information as needed to experiment with the component
  in the playground.
-->

<details>
<summary>Copy-paste this code in
<code>playground/Playground.tsx</code>:</summary>

```jsx
import React from 'react';
import {Page} from '../src';

export function Playground() {
  return (
    <Page title="Playground">
      {/* Add the code you want to test in here */}
    </Page>
  );
}
```

</details>

### 🎩 checklist

- [ ] Tested on
[mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing)
- [x] Tested on [multiple
browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers)
- [x] Tested for
[accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md)
- [x] Updated the component's `README.md` with documentation changes
- [x] [Tophatted
documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md)
changes in the style guide
juzser pushed a commit to juzser/polaris that referenced this pull request Jul 27, 2023
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## @shopify/polaris-icons@7.6.0

### Minor Changes

- [Shopify#9777](Shopify#9777)
[`8228de0f6`](Shopify@8228de0)
Thanks [@heyjoethomas](https://github.com/heyjoethomas)! - Added social
media icons

## @shopify/polaris@11.9.0

### Minor Changes

- [Shopify#9761](Shopify#9761)
[`ce3e516a2`](Shopify@ce3e516)
Thanks [@aveline](https://github.com/aveline)! - Added `readOnly` prop
to `Labelled` component


- [Shopify#9770](Shopify#9770)
[`571acc166`](Shopify@571acc1)
Thanks [@zaquille-oneil](https://github.com/zaquille-oneil)! - Updating
TextField to support ArrowUp and ArrowDown keypresses for "integer" type

### Patch Changes

- [Shopify#9765](Shopify#9765)
[`541e5920b`](Shopify@541e592)
Thanks [@mattkubej](https://github.com/mattkubej)! - [TopBar] convert to
grid and center align search field

- Updated dependencies
\[[`8228de0f6`](Shopify@8228de0)]:
    -   @shopify/polaris-icons@7.6.0

## @shopify/polaris-cli@0.2.24



## polaris.shopify.com@0.56.2

### Patch Changes

- Updated dependencies
\[[`ce3e516a2`](Shopify@ce3e516),
[`8228de0f6`](Shopify@8228de0),
[`571acc166`](Shopify@571acc1),
[`541e5920b`](Shopify@541e592)]:
    -   @shopify/polaris@11.9.0
    -   @shopify/polaris-icons@7.6.0

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
AnnaCheba pushed a commit to AnnaCheba/polaris that referenced this pull request Apr 22, 2024
… keys (Shopify#9770)

### WHY are these changes introduced?

Fixes an issue where TextFields that have the prop `type="integer"` do
not properly handle `onKeyDown` events for `ArrowUp` and `ArrowDown`.
Ideally, we increment these values by 1 multiple of the `step` value.

Context: While working on the **Admin -> Products -> Catalogs** surface,
we have a need for the `TextField` component to be able to handle
`integer` inputs while also providing the ability to increment upward
and downward with keypresses of `ArrowUp` and `ArrowDown`.

Demo after the fix is applied:
https://screenshot.click/20-27-irbo7-6glak.mp4
### WHAT is this pull request doing?
Resolves Shopify#9779
The integer input type is not a standard input type in HTML5. HTML5 does
not provide a specific input type specifically for integers.

[the
web](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input)
also doesn’t think that HTML5 inputs should take integer. if that’s the
case, then it defaults to "text" . However, polaris textFields allow for
an “integer” type when they [create input
elements](https://github.com/Shopify/polaris/blob/e10fa512d9c07ae7049d8ab4a644dbdd2114f3a6/polaris-react/src/components/TextField/TextField.tsx#L540C1-L540C1).
[this pr](Shopify#9051) has some context
on why “integer” was introduced. here is some TextFields.tsx code that i
pseudo-coded to shorten:
```
type Type =
  | 'number'
  | 'integer'
  | ...
...
interface TextFieldProps {
  inputType?: Type;
  ...
}
...
const input = createElement('input', {
  type: inputType,
  ...
}
...
```

Since `<TextField type="integer" />` is essentially `<input type="text"
/>`, TextField.tsx will need to be responsible for handling some of the
behavior that comes for free with `"number"` inputs. Some logic already
exists but I noticed that "ArrowUp" and "ArrowDown" are specifically not
being captured.

<!-- ℹ️ Delete the following for small / trivial changes -->

### How to 🎩

🖥 [Local development
instructions](https://github.com/Shopify/polaris/blob/main/README.md#local-development)
🗒 [General tophatting
guidelines](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md)
📄 [Changelog
guidelines](https://github.com/Shopify/polaris/blob/main/.github/CONTRIBUTING.md#changelog)

<!--
  Give as much information as needed to experiment with the component
  in the playground.
-->

<details>
<summary>Copy-paste this code in
<code>playground/Playground.tsx</code>:</summary>

```jsx
import React from 'react';
import {Page} from '../src';

export function Playground() {
  return (
    <Page title="Playground">
      {/* Add the code you want to test in here */}
    </Page>
  );
}
```

</details>

### 🎩 checklist

- [ ] Tested on
[mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing)
- [x] Tested on [multiple
browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers)
- [x] Tested for
[accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md)
- [x] Updated the component's `README.md` with documentation changes
- [x] [Tophatted
documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md)
changes in the style guide
ascherkus pushed a commit to ascherkus/polaris that referenced this pull request Feb 19, 2025
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## @shopify/polaris-icons@7.6.0

### Minor Changes

- [Shopify#9777](Shopify#9777)
[`8228de0f6`](Shopify@920f1c3)
Thanks [@heyjoethomas](https://github.com/heyjoethomas)! - Added social
media icons

## @shopify/polaris@11.9.0

### Minor Changes

- [Shopify#9761](Shopify#9761)
[`ce3e516a2`](Shopify@4ac8815)
Thanks [@aveline](https://github.com/aveline)! - Added `readOnly` prop
to `Labelled` component


- [Shopify#9770](Shopify#9770)
[`571acc166`](Shopify@6ff428e)
Thanks [@zaquille-oneil](https://github.com/zaquille-oneil)! - Updating
TextField to support ArrowUp and ArrowDown keypresses for "integer" type

### Patch Changes

- [Shopify#9765](Shopify#9765)
[`541e5920b`](Shopify@7040c61)
Thanks [@mattkubej](https://github.com/mattkubej)! - [TopBar] convert to
grid and center align search field

- Updated dependencies
\[[`8228de0f6`](Shopify@920f1c3)]:
    -   @shopify/polaris-icons@7.6.0

## @shopify/polaris-cli@0.2.24



## polaris.shopify.com@0.56.2

### Patch Changes

- Updated dependencies
\[[`ce3e516a2`](Shopify@4ac8815),
[`8228de0f6`](Shopify@920f1c3),
[`571acc166`](Shopify@6ff428e),
[`541e5920b`](Shopify@7040c61)]:
    -   @shopify/polaris@11.9.0
    -   @shopify/polaris-icons@7.6.0

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[TextField] allow type "integer" to handle arrowUp and arrowDown keypresses

3 participants