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

Use singular resource name for error explanation #1649

Merged
merged 1 commit into from
May 22, 2020
Merged

Use singular resource name for error explanation #1649

merged 1 commit into from
May 22, 2020

Conversation

efatsi
Copy link
Contributor

@efatsi efatsi commented May 14, 2020

Just a little thing I noticed.

Was:
Screen Shot 2020-05-14 at 11 40 06 AM

Now:
Screen Shot 2020-05-14 at 11 43 17 AM


Not sure if this is too much of a bandaid approach here. Was imagining potentially passing in the count option to display_resource_name from the error partial view code, but if you haven't defined a translation, then the default string that's constructed is what ends up being returned. Given that the default comes from default_resource_name (which pluralizes the model name), the count is often ignored for the plural version.

Thoughts on if this is good enough or if a larger refactor around how display_resource_name is used is called for?

Copy link
Collaborator

@pablobm pablobm left a comment

Choose a reason for hiding this comment

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

There has been talk of a larger refactor, but it may still take a while. Perhaps for now this band-aid is ok. @nickcharlton, do you have any thoughts?

@nickcharlton
Copy link
Member

Yeah — I think this is fine for now and ensures it reads properly. Thanks!

@nickcharlton nickcharlton merged commit d12975c into thoughtbot:master May 22, 2020
@nickcharlton nickcharlton added bug breakages in functionality that is implemented i18n translations and language support labels May 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug breakages in functionality that is implemented i18n translations and language support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants