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

Switch component #620

Merged
merged 6 commits into from
Oct 17, 2019
Merged

Conversation

adekbadek
Copy link
Contributor

@adekbadek adekbadek commented Oct 15, 2019

Fixes https://github.com/aragon/aragon-ui/issues/596

Adds a Switch component and updates theme's control-related colors.

@auto-assign auto-assign bot requested a review from bpierre October 15, 2019 17:02
@CLAassistant
Copy link

CLAassistant commented Oct 15, 2019

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@bpierre bpierre left a comment

Choose a reason for hiding this comment

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

Smooth ✨✨✨

I left a few comments, let me know what you think!

src/components/Switch/Switch.js Show resolved Hide resolved
src/components/Switch/Switch.js Outdated Show resolved Hide resolved
src/components/Switch/Switch.js Outdated Show resolved Hide resolved
src/components/Switch/Switch.js Outdated Show resolved Hide resolved
src/components/Switch/Switch.js Outdated Show resolved Hide resolved
src/components/Switch/Switch.js Outdated Show resolved Hide resolved
devbox/apps/Switch.js Outdated Show resolved Hide resolved
src/components/Switch/Switch.js Outdated Show resolved Hide resolved
src/components/Switch/Switch.js Show resolved Hide resolved
@sohkai
Copy link
Contributor

sohkai commented Oct 16, 2019

Btw @adekbadek, it would be really great to have you sign the CLA (https://github.com/aragon/aragon-ui/pull/620#issuecomment-542311208) :).

@bpierre bpierre requested review from dizzypaty and a user October 16, 2019 13:00
Copy link
Contributor

@bpierre bpierre left a comment

Choose a reason for hiding this comment

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

✔️ ✔️ ✔️

One last thing: I remember a comment from @dizzypaty about the transition being a bit slow? Maybe it has been addressed already, but if not we could try springs.swift to make it faster maybe?

Copy link
Contributor

@sohkai sohkai left a comment

Choose a reason for hiding this comment

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

Looking really good!

I have a few small, super quick things, and some discussion points.

devbox/apps/Switch.js Outdated Show resolved Hide resolved
src/components/Switch/README.md Show resolved Hide resolved

### `onChange`

- Type: `Function`: `(checked: Boolean) -> *`
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice! We should definitely do this for our other argument props :)

src/components/Switch/Switch.js Show resolved Hide resolved
src/theme/theme-light.js Show resolved Hide resolved
src/components/Switch/Switch.js Show resolved Hide resolved
src/components/Switch/README.md Show resolved Hide resolved
Copy link
Contributor

@sohkai sohkai left a comment

Choose a reason for hiding this comment

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

LGTM, let's merge!

@bpierre bpierre merged commit b02dec0 into aragon:newstyle Oct 17, 2019
bpierre pushed a commit that referenced this pull request Nov 18, 2019
Add a Switch component and update theme’s control-related colors.
bpierre pushed a commit that referenced this pull request Nov 18, 2019
Add a Switch component and update theme’s control-related colors.
bpierre pushed a commit that referenced this pull request Nov 20, 2019
Add a Switch component and update theme’s control-related colors.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Switch component
4 participants