-
Notifications
You must be signed in to change notification settings - Fork 43
Fix: item count number is not correctly shown for Quick start catalog … #413
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
Fix: item count number is not correctly shown for Quick start catalog … #413
Conversation
✅ Deploy Preview for quickstarts ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
@nicolethoen It may be worth considering an extension of the Below is a list of the files where |
|
@charlesmulder what sort of improvement to the helper function are you envisioning? |
|
@nicolethoen Yes exactly. Doing so will eliminate the need for additional // current
getResource("{{count, number}} item", quickStartsCount).replace('{{count, number}}', quickStartsCount)
// proposed
getResource("{{count, number}} item", quickStartsCount)Even more intuitive would be to adopt a more conventional getResource("%d item", quickStartsCount); // specifier is an int
getResource("%s item", quickStartsCount); // specifier is a string |
|
As long as we are prepared to handle any usages of getResource() which may not use the same placeholder formatting, then I have no objections to that. We probably cannot replace the {{ count, number }} formatting as that is part of the preexisting API that products are leveraging to format their content for the quickstarts. That would be considered a breaking change and PF avoids that unless absolutely necessary. |
|
Oh yes, that makes sense. That probably means code outside of your control is already relying on the I don’t want to complicate things, and this definitely shouldn’t form part of this PR. Thanks for the clarification and for sharing your insights! |
|
🎉 This PR is included in version 6.3.0-prerelease.8 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Fix for #412
The
getResourcefunction handles pluralization by referring to a JSON hash maphttps://github.com/patternfly/patternfly-quickstarts/blob/main/packages/module/src/locales/en/quickstart.json
I am assuming
{{ count, number }}is indicating thatcountshould be of typenumber.