Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[IndexTable] Remove in your store from IndexTable bulk selection #4013

Merged
merged 23 commits into from
Mar 3, 2021

Conversation

lhoffbeck
Copy link
Contributor

@lhoffbeck lhoffbeck commented Feb 22, 2021

WHY are these changes introduced?

Fixes shopify/web issue #38565

The IndexTable component assumes that the resource present in the list is a root-level resource, which means that the messaging when selecting all items in the list assumes that you're selecting all of that resource type across the entire store. This assumption may not always hold, which is what we're bumping up against in web.

WHAT is this pull request doing?

This PR removes the in your store assumption from the IndexTable bulk action strings.

🎩 checklist

  • Tested on mobile
  • Tested on multiple browsers
  • Tested for accessibility
  • Updated the component's README.md with documentation changes
  • Tophatted documentation changes in the style guide
  • For visual design changes, pinged one of @ HYPD, @ mirualves, @ sarahill, or @ ry5n to update the Polaris UI kit

@lhoffbeck lhoffbeck requested a review from a team February 22, 2021 16:55
@github-actions
Copy link
Contributor

github-actions bot commented Feb 22, 2021

🟢 No significant changes to src/**/*.tsx were detected.

@kyledurand
Copy link
Contributor

Does this actually improve the UX of the component? What if we removed in your store from those translation keys instead? Would the user still be able to assume the correct context?

Just trying to maintain simplicity in the code base and translations that we get locked into.

@lhoffbeck
Copy link
Contributor Author

@kyledurand this makes sense to me, double checking with the product person that asked me to make the change

@lhoffbeck lhoffbeck force-pushed the lh-adapt-index-table-to-take-parent-resource-name branch from 1cbc24a to 092264a Compare February 24, 2021 19:33
@lhoffbeck lhoffbeck changed the title [IndexTable] Add optional parent resource name [IndexTable] Remove in your store from IndexTable bulk selection Feb 24, 2021
Copy link
Contributor

@kyledurand kyledurand left a comment

Choose a reason for hiding this comment

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

👏

@lhoffbeck lhoffbeck merged commit 69928e0 into main Mar 3, 2021
@lhoffbeck lhoffbeck deleted the lh-adapt-index-table-to-take-parent-resource-name branch March 3, 2021 18:04
sylvhama pushed a commit that referenced this pull request Mar 26, 2021
…4013)

* Remove parent resource name from select all items strings

* fix test

* Update locales/zh-TW.json

* Update locales/ja.json

* Update locales/zh-CN.json

* Update locales/es.json

* Update locales/nl.json

* Update locales/fr.json

* Update locales/ko.json

* Update locales/cs.json

* Update locales/pt-PT.json

* Update locales/sv.json

* Update locales/de.json

* Update locales/it.json

* Update locales/da.json

* Update locales/pl.json

* Update locales/tr.json

* Update locales/vi.json

* Update locales/nb.json

* Update locales/fi.json

* Update locales/pt-BR.json

* Update locales/th.json

Co-authored-by: translation-platform <35075727+translation-platform@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.

None yet

3 participants