Skip to content
This repository was archived by the owner on Sep 30, 2025. It is now read-only.

Conversation

LA1CH3
Copy link
Contributor

@LA1CH3 LA1CH3 commented Mar 29, 2024

WHY are these changes introduced?

Closes #11803

Fixes an issue where the loading prop on IndexTable would not trigger the "loading panel" to appear. This was due to the CSSTransition component not functioning as expected and not applying expected CSS classes.

I opted to ditch CSSTransition altogether and just directly apply the transitions in CSS for a simpler approach.

WHAT is this pull request doing?

Example in web

  • Make price changes to any variants in the "variants" card and save the product
  • Observe that the "Loading variants..." panel appears after save, indicating reloading/refreshing of variants. It should disappear once reloading is complete.
Example video from storybook
storybook-loading.mp4

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

🎩 checklist

@LA1CH3 LA1CH3 self-assigned this Mar 29, 2024
@LA1CH3 LA1CH3 force-pushed the fix/react/index-table-loading-transition branch 2 times, most recently from b9a3ca6 to 43dba4d Compare March 29, 2024 17:15
@LA1CH3
Copy link
Contributor Author

LA1CH3 commented Mar 29, 2024

/snapit

Copy link
Contributor

🫰✨ Thanks @LA1CH3! Your snapshots have been published to npm.

Test the snapshots by updating your package.json with the newly published versions:

yarn add @shopify/polaris-migrator@0.0.0-snapshot-20240329172520
yarn add @shopify/polaris@0.0.0-snapshot-20240329172520
yarn add @shopify/stylelint-polaris@0.0.0-snapshot-20240329172520

@LA1CH3 LA1CH3 marked this pull request as ready for review March 29, 2024 17:53
@LA1CH3 LA1CH3 requested a review from yurm04 March 29, 2024 17:53
@LA1CH3 LA1CH3 force-pushed the fix/react/index-table-loading-transition branch from 43dba4d to d466758 Compare March 29, 2024 18:44
@aaronccasanova aaronccasanova force-pushed the fix/react/index-table-loading-transition branch from d466758 to 2a23a3f Compare April 19, 2024 21:52
@github-actions github-actions bot added the cla-needed Added by a bot. Contributor needs to sign the CLA Agreement. label Apr 19, 2024
@translation-platform
Copy link
Contributor

Localization quality issues found

The following issues may affect the quality of localized translations if they are not addressed:

  • The value Clear for key Polaris.ActionList.SearchField.clearButtonLabel is very short. Short strings are more likely to be misunderstood by translators without context. Please provide additional context for the translators if possible.
  • The value Search for key Polaris.ActionList.SearchField.search is very short. Short strings are more likely to be misunderstood by translators without context. Please provide additional context for the translators if possible.
  • The value Search actions for key Polaris.ActionList.SearchField.placeholder is very short. Short strings are more likely to be misunderstood by translators without context. Please provide additional context for the translators if possible.

Please look out for other instances of this issue in your PR and fix them as well if possible.

Questions about these messages? Hop in the #help-localization Slack channel.

@remy727
Copy link

remy727 commented Apr 22, 2024

Any update on this PR?

@sam-b-rose
Copy link
Member

Tested and the changes look good to me! Happy to merge this in once the conflict are resolved 👍

@LA1CH3
Copy link
Contributor Author

LA1CH3 commented Apr 23, 2024

Hey @sam-b-rose I know @yurm04 had another PR up trying to solve this (#11806) which is why there hadn't been much activity on this. However, her PR eliminates the animation while this PR keeps it intact. If we're going with this one, I will work on resolving conflicts now.

@LA1CH3 LA1CH3 force-pushed the fix/react/index-table-loading-transition branch from 2a23a3f to b9acb12 Compare April 23, 2024 14:13
@github-actions github-actions bot removed the cla-needed Added by a bot. Contributor needs to sign the CLA Agreement. label Apr 23, 2024
@LA1CH3 LA1CH3 force-pushed the fix/react/index-table-loading-transition branch from b9acb12 to e654234 Compare April 23, 2024 16:09
Copy link
Contributor

@yurm04 yurm04 left a comment

Choose a reason for hiding this comment

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

This is the quickest way to restoring the loading state and the animation, with the tradeoff being we move away from the (seemingly) defacto pattern for animations in the design system.

I think this is a fair tradeoff. @laurkim and I spent several hours trying to debug the CSSTransition component in this PR and could not quite fix the animation. Let's get this in and if we feel strongly about keeping the established pattern, we can make a ticket to capture that and pick it up when we have the bandwidth

@LA1CH3 LA1CH3 merged commit 0a9b727 into main Apr 24, 2024
@LA1CH3 LA1CH3 deleted the fix/react/index-table-loading-transition branch April 24, 2024 14:33
sophschneider pushed a commit that referenced this pull request Apr 24, 2024
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@13.2.0

### Minor Changes

- [#11535](#11535)
[`bcd16df24`](bcd16df)
Thanks [@ShabanaRumane](https://github.com/ShabanaRumane)! - Added
support for setting `maxHeight` and `minHeight` on `Popover.Pane` and
`Combobox`


- [#11907](#11907)
[`45308c97a`](45308c9)
Thanks [@zakwarsame](https://github.com/zakwarsame)! - Added an optional
`fiterLabel` prop to `ActionList` to allow for a custom placeholder

### Patch Changes

- [#11897](#11897)
[`a83084b3b`](a83084b)
Thanks [@jesstelford](https://github.com/jesstelford)! - Fixed edges of
disabled `IndexTable.Row` `Checkbox` triggering selection


- [#11924](#11924)
[`5ec70e688`](5ec70e6)
Thanks [@jesstelford](https://github.com/jesstelford)! - Upgrade to jest
29


- [#11929](#11929)
[`9ee700be6`](9ee700b)
Thanks [@sophschneider](https://github.com/sophschneider)! - Rounded
`Navigation` at `mdDown` behind a feature flag


- [#11923](#11923)
[`ce13c4366`](ce13c43)
Thanks [@jesstelford](https://github.com/jesstelford)! - Update dev
dependency: `postcss-import@^15.1.0` -> `postcss-import@^16.1.0`


- [#11925](#11925)
[`364ada59e`](364ada5)
Thanks [@sophschneider](https://github.com/sophschneider)! - Updated
Frame to only apply rounded Frame when passed a `topBar`


- [#11734](#11734)
[`1fef06256`](1fef062)
Thanks [@jesstelford](https://github.com/jesstelford)! - Upgrade to
Storybook v8


- [#11898](#11898)
[`1539f0e7c`](1539f0e)
Thanks [@jesstelford](https://github.com/jesstelford)! - Removed extra
padding around `IndexTable.Row` `Checkbox`


- [#11927](#11927)
[`5a32a3ff6`](5a32a3f)
Thanks [@sophschneider](https://github.com/sophschneider)! - Added
`prefers-reduced-motion` media queries to `Frame` width transitions


- [#11930](#11930)
[`b111629d7`](b111629)
Thanks [@jesstelford](https://github.com/jesstelford)! - Migrate
Storybook stories to CSF v3


- [#11805](#11805)
[`0a9b72721`](0a9b727)
Thanks [@LA1CH3](https://github.com/LA1CH3)! - Fixed `IndexTable`
`loading` prop to correctly show/hide loading UI when prop value changes

- Updated dependencies
\[[`5ec70e688`](5ec70e6)]:
    -   @shopify/polaris-icons@9.0.1
    -   @shopify/polaris-tokens@9.0.1

## @shopify/polaris-icons@9.0.1

### Patch Changes

- [#11924](#11924)
[`5ec70e688`](5ec70e6)
Thanks [@jesstelford](https://github.com/jesstelford)! - Upgrade to jest
29

## @shopify/polaris-migrator@1.0.1

### Patch Changes

- [#11924](#11924)
[`5ec70e688`](5ec70e6)
Thanks [@jesstelford](https://github.com/jesstelford)! - Upgrade to jest
29


- [#11930](#11930)
[`b111629d7`](b111629)
Thanks [@jesstelford](https://github.com/jesstelford)! - Migrate
Storybook stories to CSF v3

- Updated dependencies
\[[`5ec70e688`](5ec70e6)]:
    -   @shopify/polaris-tokens@9.0.1
    -   @shopify/stylelint-polaris@16.0.1

## @shopify/polaris-tokens@9.0.1

### Patch Changes

- [#11924](#11924)
[`5ec70e688`](5ec70e6)
Thanks [@jesstelford](https://github.com/jesstelford)! - Upgrade to jest
29

## @shopify/stylelint-polaris@16.0.1

### Patch Changes

- Updated dependencies
\[[`5ec70e688`](5ec70e6)]:
    -   @shopify/polaris-tokens@9.0.1

## polaris-for-vscode@1.0.1

### Patch Changes

- Updated dependencies
\[[`5ec70e688`](5ec70e6)]:
    -   @shopify/polaris-tokens@9.0.1

## polaris.shopify.com@1.0.4

### Patch Changes

- [#11924](#11924)
[`5ec70e688`](5ec70e6)
Thanks [@jesstelford](https://github.com/jesstelford)! - Upgrade to jest
29

- Updated dependencies
\[[`a83084b3b`](a83084b),
[`5ec70e688`](5ec70e6),
[`9ee700be6`](9ee700b),
[`bcd16df24`](bcd16df),
[`ce13c4366`](ce13c43),
[`364ada59e`](364ada5),
[`1fef06256`](1fef062),
[`45308c97a`](45308c9),
[`1539f0e7c`](1539f0e),
[`5a32a3ff6`](5a32a3f),
[`b111629d7`](b111629),
[`0a9b72721`](0a9b727)]:
    -   @shopify/polaris@13.2.0
    -   @shopify/polaris-icons@9.0.1
    -   @shopify/polaris-tokens@9.0.1

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@FrankHeijden
Copy link

Hey, I think this PR did something that broke the loading state on IndexTable's. For me, v13.1.2 works nicely with the loading state, but with v13.2.0-v13.4.0 the loading state seems to be always present, no matter if the loading prop is even being specified.

@yurm04
Copy link
Contributor

yurm04 commented May 29, 2024

@FrankHeijden 👋 Thanks for reaching out. Would you mind creating an issue and linking it to this PR so that we can take a look? Thank you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] IndexTable loading prop does not always re-render loading UI when changed
5 participants