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

Copy to Clipboard animation #7176

Merged

Conversation

michalbe
Copy link
Contributor

@michalbe michalbe commented Feb 11, 2017

Test Plan:

  1. Visit about:brave
  2. Click Copy to clipboard
  3. Verify copy to clipboard button now shows confirmation label when data is copied.

Details

Fixes #6297

  • 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).

New <ClipboardButton /> element was created for this.

copy-to-cb

@luixxiul
Copy link
Contributor

Great work! Adding another Less file is however not recommended based on our style guideline, Would you please modify the code based on Aphrodite? Thanks!

https://github.com/brave/browser-laptop/wiki/Refactoring-styles-to-Aphrodite

@michalbe michalbe force-pushed the issue-6297-copy-to-clipboard-animation branch from faa1db2 to 6b665ad Compare February 11, 2017 11:08
@michalbe
Copy link
Contributor Author

Fixed.
It would be nice to fix the docs you linked because for instance the first snippet:

const {StyleSheet, css} = require('aphrodite')

...

render () {
  return <a className={css(styles.orangeLink)} href='https://brave.com' />
}

...

const styles = Stylesheet.create({
  orangeLink: {
    color: 'orange',
    textDecoration: 'underline'
  }
})

doesn't work - camelcased StyleSheet was imported and capitalized Stylesheet was called later on.

@michalbe
Copy link
Contributor Author

BTW, is there a public slack or IRC for contributors? I have plenty of questions and using community.brave.com or GH isn't exactly what I'm looking for.

@bsclifton
Copy link
Member

@michalbe absolutely- we have a community Slack 😄 Please shoot me an email at clifton@brave.com and I'll make sure you get an invite

@bsclifton bsclifton added this to the 0.13.4 milestone Feb 12, 2017
Copy link
Contributor

@cezaraugusto cezaraugusto left a comment

Choose a reason for hiding this comment

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

This looks super cool. Loved the transition, thanks!

Do you mind migrating clipboardButton component to app/renderer/components instead of js/components? This way we can match our new directory structure

Otherwise, LGTM ++

@michalbe michalbe force-pushed the issue-6297-copy-to-clipboard-animation branch from 6b665ad to e3537ed Compare February 13, 2017 02:05
@michalbe
Copy link
Contributor Author

@cezaraugusto should work as expected now, thanks, I missed those docs somehow.

Copy link
Contributor

@cezaraugusto cezaraugusto left a comment

Choose a reason for hiding this comment

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

++

@bsclifton
Copy link
Member

Awesome job, @michalbe 😄 We should be able to use this component anywhere in the app!

@bsclifton bsclifton added accessibility polish Nice to have — usually related to front-end/visual tasks. release-notes/include labels Feb 15, 2017
@bsclifton bsclifton merged commit f21a71a into brave:master Feb 15, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accessibility polish Nice to have — usually related to front-end/visual tasks. QA/test-plan-specified
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants