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 implemented #203

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

Conversation

brunoluigi
Copy link
Contributor

Implemented Switch component base in the ShadCn styles but using the pseudo-classes to style the componente based in the checked state of the hidden input.

One design decision that is up to debate is the inclusion of the flag include_hidden inspired by the Rail's checkbox implementation to avoid sending no value on form submission when the switch is off.

Here are the examples to ruby-ui/web:
Screenshot 2024-11-22 at 12 32 01
Screenshot 2024-11-22 at 12 32 20

Copy link
Contributor

@stephannv stephannv left a comment

Choose a reason for hiding this comment

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

Great work. Very nice that you didn't need to use javascript for this component 🥳.

module RubyUI
class Switch < Base
def view_template
attrs => { include_hidden:, **input_attrs }
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can move this to initializer and allow to pass the checked and unchecked values, what do you think?

def initialize(include_hidden: true, checked_value: "1", unchecked_value: "0", **attrs)
  @include_hidden = include_hidden
  @checked_value = checked_value
  @unchecked_value = unchecked_value
  super(**attrs)
end

Comment on lines +21 to +27
def default_attrs
{
class: "hidden peer",
type: "checkbox",
include_hidden: true,
value: "1"
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should not allow the user to change the type and classes of the hidden checkbox, because this could break the component behavior, so this method wouldn't be necessary and we could write something like:

input(**attrs.merge(type: "checkbox", class: "hidden peer", value: @checked_value))

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