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

Adjust submit labels to use built-in i18n handling #2257

Merged
merged 7 commits into from Jun 12, 2019

Conversation

Projects
None yet
2 participants
@gravitystorm
Copy link
Collaborator

commented Jun 12, 2019

I noticed this while working on the diary entries, so now I'm rolling it out more widely. In many cases we've been avoiding using the built-in translation keys for submit buttons, and hard-coding translations based on the file the form is in (rather than based on the model for the form). This has made some form refactoring harder than it needs to be, as well as just being a bit of a waste of time. Sometimes this has been done just because of copying other places where it was done first.

There's ample opportunity to improve these labels in the future - too many saves and not enough updates in my opinion! But I'll leave the strings mostly unchanged for now.

@Nikerabbit this PR contains a few translation key renames:

  • diary_entries.show.save_button -> helpers.submit.diary_comment.create
  • messages.new.send_button -> helpers.submit.message.create
  • oauth_clients.new.submit -> helpers.submit.client_application.create
  • oauth_clients.edit.submit -> helpers.submit.client_application.update
  • redactions.new.submit -> helpers.submit.redaction.create
  • redactions.edit.submit -> helpers.submit.redaction.update
  • traces.new.upload_button -> helpers.submit.trace.create
  • traces.edit.save_button -> helpers.submit.trace.update
  • user_blocks.new.submit -> helpers.submit.user_block.create
  • user_blocks.edit.submit -> helpers.submit.user_block.update

gravitystorm added some commits Jun 12, 2019

Use built-in form sumbit translation keys for diary comments
Also use a diary comment object as the basis for the form
Use i18n for submit button for issue comments
Normally when an en-GB.yml translation is missing, rails falls back to en.yml. But when
using the submit helpers, if the en-GB translation is missing, rails knows how to create
a fallback like 'Create {model_name}' without touching the en.yml file. This string might
then be different from what the test expects, e.g. 'Add Comment'.

So it's important to set the language headers, to avoid phantomjs from picking up your desktop
preferences in this specific case.
# We don't want this to happen during the tests!
setup do
page.driver.add_headers("Accept-Language" => "en")
end

This comment has been minimized.

Copy link
@tomhughes

tomhughes Jun 12, 2019

Member

As much as we may want this I think it at least needs to be in a separate commit... I assume it got added accidentally with something else?

This comment has been minimized.

Copy link
@gravitystorm

gravitystorm Jun 12, 2019

Author Collaborator

It was deliberately in this commit, since the change to the issue comments submit button exposed the problem with the language handling in the system tests. So I thought it was useful to put them both in the same commit to fully illustrate why I was doing it and what sort of test it affects - but I can break it into two commits if you prefer.

This comment has been minimized.

Copy link
@tomhughes

tomhughes Jun 12, 2019

Member

Ah ok. I hadn't actually even worked out which commit it came from, I was just surprised to see it.

@tomhughes tomhughes merged commit a08b65a into openstreetmap:master Jun 12, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 88.39%
Details

@gravitystorm gravitystorm deleted the gravitystorm:submit_labels branch Jun 13, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.