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

Invites: Improves translated string for invite header #582

Merged
merged 3 commits into from
Nov 24, 2015

Conversation

ebinnion
Copy link
Contributor

Addresses part of #138 and related to #369

To test:

  • Checkout update/invite-header-role-strings branch
  • Invite a test user to join a WordPress.com site by going to Users > Invite New in wp-admin
  • In the invitation email, make note of the invitation key
  • Go to /accept-invite/$site/$invite_key
  • Does the "invited you..." text seem appropriate for the role?
  • Now, go back to the invite users UI in wp-admin
  • Invite the same user, but with a different role this time
  • Refresh the tab that has Calypso loaded
  • Do the strings represent the new role?

cc @roccotripaldi for review and @rickybanister for wording review

@ebinnion ebinnion added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Status] Needs Design Review Add this when you'd like to get a review / feedback from the Design team on your PR People Management labels Nov 23, 2015
@ebinnion ebinnion self-assigned this Nov 23, 2015
@ebinnion ebinnion added this to the People Management: m6 milestone Nov 23, 2015
break;
case 'author':
text = this.translate(
'{{inviterName/}} invited you to author on:', {
Copy link
Member

Choose a reason for hiding this comment

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

invited you to be an author on:

@rickybanister
Copy link

They all seem great except this is weird:

invited you to author on

Let's go with

invited you to be an author on:

It doesn't match the others if we don't use 'author' as a verb, but I don't feel it's commonly used as a verb these days.

switch ( get( this.props, 'invite.meta.role' ) ) {
case 'administrator':
text = this.translate(
'{{inviterName/}} invited you to administer:', {
Copy link
Member

Choose a reason for hiding this comment

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

User invited you to administer: feels a bit strange. I'd prefer User invited you to manage:

@roccotripaldi
Copy link
Member

Code looks great, and works as expected. I left one minor wording fix.

ebinnion added a commit that referenced this pull request Nov 24, 2015
…ings

Invites: Improves translated string for invite header
@ebinnion ebinnion merged commit 6a20952 into master Nov 24, 2015
@scruffian scruffian removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Nov 24, 2015
@ebinnion ebinnion deleted the update/invite-header-role-strings branch November 24, 2015 17:59
@rickybanister
Copy link

It appears this is already merged, but I don't think we should have switched to use the word 'manage'.

The role is 'admin' so I think using some derivation of that term is important.

has invited you to be an admin on: would be better than invited you to manage I think.

@lancewillett lancewillett removed the [Status] Needs Design Review Add this when you'd like to get a review / feedback from the Design team on your PR label Oct 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
People Management [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants