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

People: Update remove follower alert text #3120

Merged
merged 3 commits into from
Feb 5, 2016

Conversation

BrookeDot
Copy link
Contributor

This PR fixed the he or she language as mentioned in #2672 (comment) which was originally committed in #2673. It updates the message shown in the confirm alert when removing a follower:
screenshot_05-02-2016-09 52 05
It also fixes a translation error on line 127.
headerText = this.props.label, instead of headerText = this.translate( this.props.label, { context: 'A navigation label.' } ),

cc: @ebinnion

@BrookeDot BrookeDot added [Type] Enhancement [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. People Management labels Feb 5, 2016
@ebinnion
Copy link
Contributor

ebinnion commented Feb 5, 2016

I went ahead and added a commit that handles some ESLint issues in FollowersList.

While it may seem wrong, removing the this.translate() in this 0e03c47 works because we are translating the label one component up in Main.jsx.

Since I made a few changes, cc @lezama for a final code review.

@lezama
Copy link
Contributor

lezama commented Feb 5, 2016

LGTM 👍

@lezama lezama added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Feb 5, 2016
ebinnion added a commit that referenced this pull request Feb 5, 2016
People: Update remove follower alert text
@ebinnion ebinnion merged commit 7ddb39b into master Feb 5, 2016
@ebinnion ebinnion deleted the update/remove-follower-alert branch February 5, 2016 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants