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

Conversation

yurm04
Copy link
Contributor

@yurm04 yurm04 commented Mar 29, 2024

WHY are these changes introduced?

Updating the loading prop was not rendering the loadingMarkup for the index table loading state

Fixes #11803

WHAT is this pull request doing?

Checking loading to conditionally render loadingMarkup

Screen.Recording.2024-03-29.at.12.14.28.PM.mov

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

🎩 checklist

@yurm04 yurm04 requested review from LA1CH3 and a team March 29, 2024 17:21
@yurm04 yurm04 changed the title Ye/index table loading [IndexTable] Conditionally render loadingMarkup on loading prop change Mar 29, 2024
@alex-page alex-page requested a review from mrcthms April 1, 2024 23:10
@LA1CH3
Copy link
Contributor

LA1CH3 commented Apr 2, 2024

Hey folks, the changes in this PR effectively eliminate the CSS transition of the loading panel. It just appears and disappears now.

I have a PR up that addresses this by just adding/removing classes and leveraging transition CSS property directly (#11805) but I know @yurm04 was planning on digging into this more to figure out what the root issue with the CSSTransition component is here.

@alex-page
Copy link
Member

Can we ship and fix the animation as a follow up? It sucks that this is not rendering anything right now.

@aaronccasanova aaronccasanova force-pushed the ye/index-table-loading branch from 9f9c4ce to 56a7fc1 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.

@yurm04
Copy link
Contributor Author

yurm04 commented Apr 24, 2024

closing in favor of #11805

@yurm04 yurm04 closed this Apr 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla-needed Added by a bot. Contributor needs to sign the CLA Agreement.
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