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

#167 Allow classnames for switch #171

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

rushi
Copy link
Member

@rushi rushi commented Aug 21, 2022

No description provided.

@rushi rushi requested a review from nemanjakrstic August 21, 2022 08:20
// TODO: Native checkbox input `checked` and `disabled` properties are not prefixed with `is`.
// Decide on the naming convention.
export const Switch = ({ isChecked = false, size = "medium", ...rest }) => {
export const Switch = ({ isChecked = false, size = "medium", classNames = { parent: "", inner: "" }, ...rest }) => {
Copy link
Member Author

Choose a reason for hiding this comment

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

parent and inner match what we 8 lines about in sizes

Copy link
Contributor

Choose a reason for hiding this comment

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

How about container and input (or switch)? That is what we used for some other components

isChecked ? sizes[size].translate : "translate-x-0",
"ui-switch-inner pointer-events-none inline-block transform rounded-full bg-white shadow ring-0 transition duration-200 ease-in-out",
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved above

Comment on lines -33 to +31
"relative inline-flex flex-shrink-0 cursor-pointer rounded-full border-2 border-transparent transition-colors duration-200 ease-in-out focus:outline-none",
"relative inline-flex flex-shrink-0 cursor-pointer rounded-full border-2 border-transparent",
Copy link
Member Author

Choose a reason for hiding this comment

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

Split into two lines

@@ -49,6 +51,7 @@ export const Switch = ({ isChecked = false, size = "medium", ...rest }) => {
Switch.propTypes = {
isChecked: PropTypes.bool,
size: PropTypes.string,
classNames: PropTypes.shape({ switch: PropTypes.string, content: PropTypes.string }),
Copy link
Contributor

Choose a reason for hiding this comment

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

Does not match the props

// TODO: Native checkbox input `checked` and `disabled` properties are not prefixed with `is`.
// Decide on the naming convention.
export const Switch = ({ isChecked = false, size = "medium", ...rest }) => {
export const Switch = ({ isChecked = false, size = "medium", classNames = { parent: "", inner: "" }, ...rest }) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

How about container and input (or switch)? That is what we used for some other components

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.

2 participants