Skip to content

Conversation

@highfieldjames
Copy link
Contributor

@highfieldjames highfieldjames commented Aug 30, 2023

WHY are these changes introduced?

Fixes https://github.com/Shopify/web/issues/102624

When a TextField Spinner is clicked on, the button is the document's active element (i.e. the button is focused). When hitting the enter key, if the TextField is in a form, the form should be submitted, but instead nothing happens. This differs from the default behaviour of an <input type="number"> where the input is focused after clicking a spinner.

See this codesandbox for a replication of the issue: https://codesandbox.io/s/cranky-rgb-ktnqml?file=/App.tsx

Note that despite the button being focused previously, hitting enter had no effect, so this PR does not break or change existing functionality, only fixes broken behaviour.

WHAT is this pull request doing?

This PR changes the TextField component so the input is focused when the stepper is clicked.

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

Tophat on Storybook

  1. Click a stepper to increase the text field numerical value
  2. Hit enter
  3. The form should submit, and the text field should reset to '0'. Previously nothing happened.

🎩 checklist

@highfieldjames highfieldjames force-pushed the jh-text-field-spinner-focus branch 2 times, most recently from e12edc2 to 61dd7d0 Compare August 30, 2023 15:29
@highfieldjames highfieldjames force-pushed the jh-text-field-spinner-focus branch from 61dd7d0 to da58318 Compare August 30, 2023 15:40
@highfieldjames highfieldjames force-pushed the jh-text-field-spinner-focus branch from da58318 to f926f19 Compare August 31, 2023 13:55
Copy link
Member

@chloerice chloerice left a comment

Choose a reason for hiding this comment

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

This makes sense, thank you for contributing @highfieldjames 🎉

@chloerice chloerice dismissed their stale review August 31, 2023 14:47

I said approve!

Copy link

@brandon-yetman brandon-yetman left a comment

Choose a reason for hiding this comment

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

✅ nice improvement!

@highfieldjames highfieldjames merged commit abb5025 into main Sep 1, 2023
@highfieldjames highfieldjames deleted the jh-text-field-spinner-focus branch September 1, 2023 13:36
kyledurand pushed a commit that referenced this pull request Sep 7, 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.9.0

### Minor Changes

- [#9856](#9856)
[`47652f7d6`](47652f7)
Thanks [@heyjoethomas](https://github.com/heyjoethomas)! - Updates
social media icons, removing them from their containers and adds a one
for the Twitch platform.

## @shopify/polaris-migrator@0.22.0

### Minor Changes

- [#10263](#10263)
[`67699cb88`](67699cb)
Thanks [@aveline](https://github.com/aveline)! - Added migration for
`Button` component

## @shopify/polaris@11.15.0

### Minor Changes

- [#9701](#9701)
[`cbf539495`](cbf5394)
Thanks [@martenbjork](https://github.com/martenbjork)! - Updated the
Frame and Topbar components to stay clear of a scrollbar. This reduces
the overall jumpiness in the UI when scrollbars appear and disappear
when using a Polaris app.

### Patch Changes

- [#10284](#10284)
[`eba75d20a`](eba75d2)
Thanks [@zakwarsame](https://github.com/zakwarsame)! - - Updated
`Filters` query field to initialize with focus based on `mode` state


- [#10282](#10282)
[`9a2d4f62a`](9a2d4f6)
Thanks [@kyledurand](https://github.com/kyledurand)! - Fixed pointer
alignment on tooltip


- [#10343](#10343)
[`12a62b4d7`](12a62b4)
Thanks [@mrcthms](https://github.com/mrcthms)! - Fixed UI
inconsistencies in the mobile view of the IndexFilters


- [#10276](#10276)
[`abb50250e`](abb5025)
Thanks [@highfieldjames](https://github.com/highfieldjames)! - Updated
`TextField` of `type` `number` to focus when a `Spinner` button is
clicked

- Updated dependencies
\[[`47652f7d6`](47652f7)]:
    -   @shopify/polaris-icons@7.9.0

## polaris.shopify.com@0.57.3

### Patch Changes

- Updated dependencies
\[[`47652f7d6`](47652f7),
[`eba75d20a`](eba75d2),
[`9a2d4f62a`](9a2d4f6),
[`12a62b4d7`](12a62b4),
[`cbf539495`](cbf5394),
[`abb50250e`](abb5025)]:
    -   @shopify/polaris-icons@7.9.0
    -   @shopify/polaris@11.15.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
<!--
  ☝️How to write a good PR title:
- Prefix it with [ComponentName] (if applicable), for example: [Button]
  - Start with a verb, for example: Add, Delete, Improve, Fix…
  - Give as much context as necessary and as little as possible
  - Prefix it with [WIP] while it’s a work in progress
-->

### WHY are these changes introduced?

Fixes Shopify/web#102624

<!--
  Context about the problem that’s being addressed.
-->

When a `TextField` `Spinner` is clicked on, the button is the document's
active element (i.e. the button is focused). When hitting the enter key,
if the `TextField` is in a form, the form should be submitted, but
instead nothing happens. This differs from the default behaviour of an
`<input type="number">` where the input is focused after clicking a
spinner.

See this codesandbox for a replication of the issue:
https://codesandbox.io/s/cranky-rgb-ktnqml?file=/App.tsx

Note that despite the button being focused previously, hitting enter had
no effect, so this PR does not break or change existing functionality,
only fixes broken behaviour.

### WHAT is this pull request doing?

This PR changes the TextField component so the `input` is focused when
the stepper is clicked.

<!-- ℹ️ 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)

[Tophat on
Storybook](https://5d559397bae39100201eedc1-wekmtcxvyp.chromatic.com/?path=/story/all-components-textfield--with-form-submit)

1. Click a stepper to increase the text field numerical value
2. Hit enter
3. The form should submit, and the text field should reset to '0'.
Previously nothing happened.


### 🎩 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.9.0

### Minor Changes

- [Shopify#9856](Shopify#9856)
[`47652f7d6`](Shopify@5443369)
Thanks [@heyjoethomas](https://github.com/heyjoethomas)! - Updates
social media icons, removing them from their containers and adds a one
for the Twitch platform.

## @shopify/polaris-migrator@0.22.0

### Minor Changes

- [Shopify#10263](Shopify#10263)
[`67699cb88`](Shopify@3584b03)
Thanks [@aveline](https://github.com/aveline)! - Added migration for
`Button` component

## @shopify/polaris@11.15.0

### Minor Changes

- [Shopify#9701](Shopify#9701)
[`cbf539495`](Shopify@f95f366)
Thanks [@martenbjork](https://github.com/martenbjork)! - Updated the
Frame and Topbar components to stay clear of a scrollbar. This reduces
the overall jumpiness in the UI when scrollbars appear and disappear
when using a Polaris app.

### Patch Changes

- [Shopify#10284](Shopify#10284)
[`eba75d20a`](Shopify@5dfc913)
Thanks [@zakwarsame](https://github.com/zakwarsame)! - - Updated
`Filters` query field to initialize with focus based on `mode` state


- [Shopify#10282](Shopify#10282)
[`9a2d4f62a`](Shopify@8b6b12e)
Thanks [@kyledurand](https://github.com/kyledurand)! - Fixed pointer
alignment on tooltip


- [Shopify#10343](Shopify#10343)
[`12a62b4d7`](Shopify@a7b16c6)
Thanks [@mrcthms](https://github.com/mrcthms)! - Fixed UI
inconsistencies in the mobile view of the IndexFilters


- [Shopify#10276](Shopify#10276)
[`abb50250e`](Shopify@31df1db)
Thanks [@highfieldjames](https://github.com/highfieldjames)! - Updated
`TextField` of `type` `number` to focus when a `Spinner` button is
clicked

- Updated dependencies
\[[`47652f7d6`](Shopify@5443369)]:
    -   @shopify/polaris-icons@7.9.0

## polaris.shopify.com@0.57.3

### Patch Changes

- Updated dependencies
\[[`47652f7d6`](Shopify@5443369),
[`eba75d20a`](Shopify@5dfc913),
[`9a2d4f62a`](Shopify@8b6b12e),
[`12a62b4d7`](Shopify@a7b16c6),
[`cbf539495`](Shopify@f95f366),
[`abb50250e`](Shopify@31df1db)]:
    -   @shopify/polaris-icons@7.9.0
    -   @shopify/polaris@11.15.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.

3 participants