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

new Toggle component #92

Merged
merged 5 commits into from
Apr 25, 2019
Merged

new Toggle component #92

merged 5 commits into from
Apr 25, 2019

Conversation

daigof
Copy link
Contributor

@daigof daigof commented Apr 24, 2019

@daigof daigof requested review from snags88 and benkolde April 24, 2019 17:07
@snags88 snags88 temporarily deployed to curology-radiance-pr-92 April 24, 2019 17:08 Inactive
@snags88 snags88 temporarily deployed to curology-radiance-pr-92 April 24, 2019 17:10 Inactive
Copy link
Contributor

@snags88 snags88 left a comment

Choose a reason for hiding this comment

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

🔥

width: 40,
};

export const thumbStyle = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add a border of 1px and make it COLORS.border to the thumb

Copy link
Contributor

Choose a reason for hiding this comment

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

And make the background color of the thumb, white.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

width: 40,
};

export const thumbStyle = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Also looks like the thumb needs to be nudged over 1px when it's toggled on and toggled off.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved it closer to edge 1 pixel when active, moving 1 more pixel closer to each edge started to feel odd for me, take a look this is one more pixel to each side

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

current
image

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay let's go with whatever is marked Current. I don't know what I was seeing

margin-right: ${SPACING.xsmall};
`;

export const trackStyle = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we make the track background color purple15? That's on me. It wasn't included in the design mocks but I think it will be better. It also matches our disabled button styling.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@snags88 snags88 temporarily deployed to curology-radiance-pr-92 April 25, 2019 16:31 Inactive
@daigof daigof merged commit c04f581 into master Apr 25, 2019
@daigof daigof deleted the toggle-component branch April 25, 2019 16:41
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.

3 participants