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

Add support for radio buttons based on a slot #136

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

Conversation

Fleker
Copy link

@Fleker Fleker commented Mar 17, 2020

For a smart home sample project we want to have several exclusive options for a given smart home state backed by icons. Rather than writing our own radio-group element we'd like to use the existing radio-group and radio-button elements. This change adds an optional icon attribute and icon slot to the element which transforms the regular empty/filled buttons into a colorful icon.

See the updated demo for the design.

image

@proppy

</style>

<div id="radioContainer">
<div id="offRadio"></div>
<div id="offRadio">
<slot name="icon"></slot>
Copy link

Choose a reason for hiding this comment

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

what about putting the slot on #radioContainer instead have have #onRadio #offRadio as the fallback content. That would allow you not override any of the existing CSS rules.

Copy link
Author

Choose a reason for hiding this comment

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

Something like

<slot name="icon">
  <p>I'm some default content!</p>
</slot>

One challenge I ran into making this PR is that it seems hard to actually change the styles in shadow DOM from outside the stylesheet. I couldn't apply changes to #offRadio unless it was in this element or I exposed a variable.

Copy link

@proppy proppy Mar 18, 2020

Choose a reason for hiding this comment

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

I was more thinking of:

<div id="radioContainer">
   <slot name="radioContent">
       <div id="onRadio"></div>
       <div id="offRadio"></div>
   </slot>
</div>

And the CSS for #onRadio and #offRadio would get automatically trashed if you override the slot.

Copy link
Author

Choose a reason for hiding this comment

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

If I put something custom in the slot, can I customize it using external CSS? Or is it still bound by shadow DOM rules?

Copy link

Choose a reason for hiding this comment

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

Assuming it's an iron-icon and the existing CSS rules that applied on #onRadio and #offRadio are discarded, would it need any additional external CSS than the one included in the <iron-icon> shadow DOM?

Copy link

@proppy proppy Mar 18, 2020

Choose a reason for hiding this comment

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

https://developer.mozilla.org/en-US/docs/Web/CSS/::part or have a custom iron-radio-selectable that embed an icon w/ dedicated CSS and follow https://www.webcomponents.org/element/@polymer/paper-radio-group selection protocol (thru PaperCheckedElementBehavior?)

Copy link
Author

Choose a reason for hiding this comment

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

For our purposes we really just need to stick an icon in there. Maybe we can use the ::part rule, but I don't want to implement too much custom behavior.

Copy link
Author

Choose a reason for hiding this comment

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

I'd be interested in getting some advice from @bicknellr on what may be the most idiomatic way

Copy link
Author

Choose a reason for hiding this comment

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

I've updated the PR to use ::part and extract the style behavior outside of the component. I'm not sure if this is the best approach, as it means the default will not work very well, but it should be otherwise be functional.

Copy link

Choose a reason for hiding this comment

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

Now that you're using ::part that should mean that the CSS could live outside of the element, no? So the only patch you really need here is a <slot> (and putting in on the parent would allow you to discard both element and their associated css rule).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants