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

[ResourceList] Fix for the plural resource name when totalItemsCount is greater than 1 #2301

Merged
merged 2 commits into from
Oct 18, 2019
Merged

[ResourceList] Fix for the plural resource name when totalItemsCount is greater than 1 #2301

merged 2 commits into from
Oct 18, 2019

Conversation

mbaumbach
Copy link
Contributor

WHY are these changes introduced?

Fixes #2299

When the ResourceList has only item showing, but a totalItemsCount greater than 1, it was using the singular form of the resource name when it should use the plural.

WHAT is this pull request doing?

Whenever there's a totalItemsCount and it's not equal to 1, it will use the plural form of the resource name.

🎩 checklist

@danrosenthal danrosenthal requested a review from dleroux October 16, 2019 18:51
Copy link
Contributor

@dleroux dleroux left a comment

Choose a reason for hiding this comment

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

Awesome! Thank you for doing this @mbaumbach.

There's a conflict with the changelog but after that, it's good to go.

@BPScott
Copy link
Member

BPScott commented Oct 17, 2019

Are the changes to eslintignore and all the components other than ResourceList desired? They all look unrelated?

@mbaumbach
Copy link
Contributor Author

@BPScott Nope, not desired. I think that may have been due to a merge with master. I thought I rebased those when trying to resolve the conflicts with UNRELEASED.md though.

I can always create a new branch with just these changes if that's preferred.

@BPScott
Copy link
Member

BPScott commented Oct 17, 2019

Rebasing this atop latest master is fine, no need for new branches and new PRs :)

@mbaumbach mbaumbach closed this Oct 17, 2019
@mbaumbach mbaumbach reopened this Oct 17, 2019
@mbaumbach
Copy link
Contributor Author

Apologies about that auto-close of the PR. Should be all set now, I think.

Copy link
Contributor

@tmlayton tmlayton left a comment

Choose a reason for hiding this comment

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

Nice one!

@tmlayton
Copy link
Contributor

Will squash and merge once the checks pass

@tmlayton tmlayton merged commit 4b01c38 into Shopify:master Oct 18, 2019
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.

[ResourceList] totalItemsCount causes singular of resource to be used when showing one resource
4 participants