Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Fix copy to clipboard tooltip #8201

Merged
merged 1 commit into from
Apr 10, 2017
Merged

Fix copy to clipboard tooltip #8201

merged 1 commit into from
Apr 10, 2017

Conversation

cndouglas
Copy link

@cndouglas cndouglas commented Apr 10, 2017

Test Plan:

  1. Open about:brave with kabob > Help > About Brave.
  2. Hover over the copy to clipboard button as shown in the screenshot below.
  3. Make sure the tooltip Copy to clipboard tooltip is shown.

Description

Fixes #8199

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).

@cndouglas cndouglas added this to the 0.14.3 milestone Apr 10, 2017
@cndouglas cndouglas self-assigned this Apr 10, 2017
@@ -37,9 +37,10 @@ class ClipboardButton extends React.Component {
<span className={css(
styles.doneLabel,
this.state.visibleLabel && styles.visible
)} onAnimationEnd={this.onAnimationEnd} data-l10n-id='copied'>Copiesd!</span>
)} onAnimationEnd={this.onAnimationEnd} data-l10n-id='copied'>Copied!</span>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we have english text hardcoded?

Copy link
Author

Choose a reason for hiding this comment

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

Not sure. I'll remove it.

@@ -37,7 +37,7 @@ class ClipboardButton extends React.Component {
<span className={css(
styles.doneLabel,
this.state.visibleLabel && styles.visible
)} onAnimationEnd={this.onAnimationEnd} data-l10n-id='copied'>Copied!</span>
)} onAnimationEnd={this.onAnimationEnd} data-l10n-id='copied'></span>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please change this to data-l10n-id='copied' />

Copy link
Author

Choose a reason for hiding this comment

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

Sure.

@cndouglas
Copy link
Author

@NejcZdovc I fixed everything you pointed out and squashed the commits.

Copy link
Contributor

@NejcZdovc NejcZdovc left a comment

Choose a reason for hiding this comment

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

LGTM, thank you

@bsclifton bsclifton modified the milestones: 0.14.2, 0.14.3 Apr 10, 2017
@bsclifton bsclifton merged commit 4885a10 into brave:master Apr 10, 2017
@cndouglas cndouglas deleted the copy-clipboard-tooltip branch April 10, 2017 20:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants