Skip to content

Conversation

@zaquille-oneil
Copy link
Contributor

@zaquille-oneil zaquille-oneil commented Oct 23, 2023

WHY are these changes introduced?

Fixes #10948

WHAT is this pull request doing?

Prevent onBlur in TextField from triggering when moving focus between the <input /> and <Spinner /> child elements within a <TextField />

How to 🎩

Spin HQ

Tophat steps:

  • Observe the buggy behavior here: https://github.com/Shopify/polaris/assets/4310197/f298a261-76ca-452b-b9f0-984d789bcd26
  • Go to the CatalogEditor for a given Catalog
  • Open the QuantitySettingsModal by hovering over a row in the table and clicking in the column underneath "Quantity rules"
  • Test against the behavior in the buggy video linked above
    • note: the expected onBlur behavior that should occur when manipulating the "Increment" TextField:
      • "Minimum" value "snaps" (adjusts) to match the minimum behavior
      • any relevant "snap" messages should appear below
    • verify that subsequent increases to the "Increment" value via the <Spinner /> does not trigger onBlur logic to occur UNTIL the user blurs off of the entire TextField component
    • verify that changes to "Increment" TextField value via the <input /> does not trigger onBlur logic to occur UNTIL the user blurs off the entire TextField component
      • e.g. set input value to "5", then, without blurring off the "Increment" TextField, click the up arrow to bump the value to "6". this should not trigger any onBlur logic

🖥 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 Zl/text field blur [TextField] Prevent onBlur from triggering when clicking between child components (spinner and input) Oct 23, 2023
@zaquille-oneil zaquille-oneil requested a review from a team October 25, 2023 19: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.

Thanks for catching this @zaquille-oneil! You'll just need to add a changeset and then this is good to go.

@chloerice chloerice marked this pull request as ready for review October 26, 2023 02:48
@chloerice chloerice added Contribution Component Bug Something is broken and not working as intended in the system. labels Oct 26, 2023
@zaquille-oneil zaquille-oneil self-assigned this Oct 26, 2023
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.

LGTM, nice job! +1 to @chloerice's suggested change before merging

Co-authored-by: Chloe Rice <chloerice@users.noreply.github.com>
@zaquille-oneil
Copy link
Contributor Author

I spoke with @highfieldjames about how these changes may affect sections/Inventory, specifically the InventoryListPage in the EditableQuantityCell component.

In this Polaris PR, I changed the behaviour of TextFields to match the default behaviour of which is to focus the input after clicking a spinner
When tophatting your PR on the inventory list page, hitting the enter key after clicking the spinner no longer submits the form. This is something we need to keep the behaviour of.

James suggested a change:

function handleOnBlur(event: React.FocusEvent) {
    setFocus(false);

    // Return early if new focus target is inside the TextField component
    if (textFieldRef.current?.contains(event?.relatedTarget)) {
      return;
    }

    if (onBlur) {
      onBlur(event);
    }
  }

I’ve just spun up a new instance to see what’s going on exactly, and returning only after setFocus has been called would allow the form submission to work as expected (not sure why though…). Does this still fix the issue you are trying to fix?

so my next step will be to make sure that this doesn't affect behavior on the Catalogs side. Will keep this thread up-to-date

Copy link
Contributor

@highfieldjames highfieldjames left a comment

Choose a reason for hiding this comment

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

All looks good for inventory adjustments! Thanks Zak! 🚀

@zaquille-oneil zaquille-oneil merged commit 38ed8a9 into main Oct 31, 2023
@zaquille-oneil zaquille-oneil deleted the ZL/text-field-blur branch October 31, 2023 15:17
sophschneider pushed a commit that referenced this pull request Oct 31, 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@12.0.3

### Patch Changes

- [#11018](#11018)
[`38ed8a9ba`](38ed8a9)
Thanks [@zaquille-oneil](https://github.com/zaquille-oneil)! - Fixed
`TextField` blurring when interacting with the `Spinner` buttons


- [#11067](#11067)
[`a1cff3557`](a1cff35)
Thanks [@laurkim](https://github.com/laurkim)! - Fixed disabled button
styling on `ContextualSaveBar` component


- [#11066](#11066)
[`9d5fedbce`](9d5fedb)
Thanks [@laurkim](https://github.com/laurkim)! - Fixed styling on
`IndexTable` component header tooltip

## polaris.shopify.com@0.60.1

### Patch Changes

- Updated dependencies
\[[`38ed8a9ba`](38ed8a9),
[`a1cff3557`](a1cff35),
[`9d5fedbce`](9d5fedb)]:
    -   @shopify/polaris@12.0.3

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@chloerice chloerice added 2023 Contribution tracking for 2023 Contribution Tracking contribution in conjuction with year and type, e.g., "Component" + "Contribution" + "2023" labels Dec 5, 2023
AnnaCheba pushed a commit to AnnaCheba/polaris that referenced this pull request Apr 22, 2024
…d components (spinner and input) (Shopify#11018)

<!--
  ☝️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#10948

### WHAT is this pull request doing?

Prevent `onBlur` in TextField from triggering when moving focus between
the `<input />` and `<Spinner />` child elements within a `<TextField
/>`
<!--
  Summary of the changes committed.

Before / after screenshots are appreciated for UI changes. Make sure to
include alt text that describes the screenshot.

If you include an animated gif showing your change, wrapping it in a
details tag is recommended. Gifs usually autoplay, which can cause
accessibility issues for people reviewing your PR:

    <details>
      <summary>Summary of your gif(s)</summary>
      <img src="..." alt="Description of what the gif shows">
    </details>
-->

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

### How to 🎩

[Spin HQ](https://web-polaris-text-field.zak-lee.us.spin.dev/)
- `web` checked out on
[ZL/quantity-price-break-minimum-quantity-fix](Shopify/web#100618)
- `polaris` checked out on this PR's branch (ZL/text-field-blur)

Tophat steps:
- Observe the buggy behavior here:
https://github.com/Shopify/polaris/assets/4310197/f298a261-76ca-452b-b9f0-984d789bcd26
- Go to the
[CatalogEditor](https://admin.web.web-polaris-text-field.zak-lee.us.spin.dev/store/shop1/catalogs/9/editor)
for a given Catalog
- Open the QuantitySettingsModal by hovering over a row in the table and
clicking in the column underneath "Quantity rules"
- Test against the behavior in the buggy video linked above
- note: the expected `onBlur` behavior that should occur when
manipulating the "Increment" TextField:
    - "Minimum" value "snaps" (adjusts) to match the minimum behavior
    - any relevant "snap" messages should appear below
- verify that subsequent increases to the "Increment" value via the
`<Spinner />` does not trigger `onBlur` logic to occur UNTIL the user
blurs off of the entire TextField component
- verify that changes to "Increment" TextField value via the `<input />`
does not trigger `onBlur` logic to occur UNTIL the user blurs off the
entire TextField component
- e.g. set input value to "5", then, without blurring off the
"Increment" TextField, click the up arrow to bump the value to "6". this
should not trigger any `onBlur` logic



🖥 [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)
- [ ] Updated the component's `README.md` with documentation changes
- [ ] [Tophatted
documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md)
changes in the style guide

---------

Co-authored-by: Chloe Rice <chloerice@users.noreply.github.com>
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@12.0.3

### Patch Changes

- [Shopify#11018](Shopify#11018)
[`38ed8a9ba`](Shopify@947fba6)
Thanks [@zaquille-oneil](https://github.com/zaquille-oneil)! - Fixed
`TextField` blurring when interacting with the `Spinner` buttons


- [Shopify#11067](Shopify#11067)
[`a1cff3557`](Shopify@71ff008)
Thanks [@laurkim](https://github.com/laurkim)! - Fixed disabled button
styling on `ContextualSaveBar` component


- [Shopify#11066](Shopify#11066)
[`9d5fedbce`](Shopify@0679052)
Thanks [@laurkim](https://github.com/laurkim)! - Fixed styling on
`IndexTable` component header tooltip

## polaris.shopify.com@0.60.1

### Patch Changes

- Updated dependencies
\[[`38ed8a9ba`](Shopify@947fba6),
[`a1cff3557`](Shopify@71ff008),
[`9d5fedbce`](Shopify@0679052)]:
    -   @shopify/polaris@12.0.3

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

2023 Contribution tracking for 2023 Bug Something is broken and not working as intended in the system. Component Contribution Tracking contribution in conjuction with year and type, e.g., "Component" + "Contribution" + "2023"

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[TextField] Adjust onBlur logic so that it does not trigger when clicking on the Spinner within the text field

4 participants