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

Change gaia-switch to accept a label as a slot #13

Open
KevinGrandon opened this issue Jun 25, 2015 · 8 comments
Open

Change gaia-switch to accept a label as a slot #13

KevinGrandon opened this issue Jun 25, 2015 · 8 comments

Comments

@KevinGrandon
Copy link
Member

Currently you can't click on labels to toggle the switch, and you should be able to do so.

The current gaia-switch component in gaia shared/ passes in the label to the component, and is a nicer way of going about things I think: https://github.com/mozilla-b2g/gaia/blob/540701791684cc527dccc9b7f1d4b694c7ce296f/apps/ftu/index.html#L262

This also works in pretty much every use case that we have for switches in gaia.

@wilsonpage
Copy link
Contributor

We do support the use of labels with a for attribute.

I'm not fan of nesting <label> inside <gaia-switch>. If we want implicit <label> -> <gaia-switch> click retargeting I think this issue is best solved properly with a <gaia-label> component that knows about which nested components (eg. <gaia-switch>, <gaia-checkbox>, etc) need special behaviour.

@KevinGrandon
Copy link
Member Author

I'll leave it up to you. Nesting labels inside of gaia-switch controls will make the transition between the existing gaia-switch in gaia and this component extremely easy.

@KevinGrandon
Copy link
Member Author

After working with many of the legacy components in gaia over the last few weeks, I'd just like to leave my experiences here.

The legacy components in gaia/shared use a nested label inside each component for the ease of porting them. It's consistent with building blocks (not really a good reason), but also many apps have custom styling the a single building block (the switch + label). I think breaking the switch component out into two pieces is going to cause a lot of extra migration work, and break things.

Do you think it would be possible to have support for both nested labels, and external labels? This would make porting the existing usage of web components and building blocks extremely simple.

@wilsonpage
Copy link
Contributor

Does the styling of these nested labels have to be included here?

@KevinGrandon
Copy link
Member Author

Does the styling of these nested labels have to be included here?

I imagine it would, but maybe you could do something tricky. Alternatively we could have another parent class, something like "ComboSwitch", but this does feel a little too verbose to me.

<switch-control>
  <gaia-label />
  <gaia-switch />
</switch-control>

@wilsonpage
Copy link
Contributor

IMO it would be simpler to just create a simple <gaia-label> and replace existing CSS building-blocks with:

<gaia-label>Title<gaia-switch/></gaia-label>

This way we define the label styling in one place to be used alongside <gaia-switch>, <gaia-checkbox> and <gaia-radio>, instead of duplicating label styles in all three components.

This approach is extensible, it reflects how built-in form controls work today and means we won't be living with the nested label hack for x years.

If you don't want the hassle of doing this, we have the for attribute which is usable today.

@KevinGrandon
Copy link
Member Author

@wilsonpage - In order to easily port gaia-components I think we need a drop-in replacement for the existing gaia-switch (and other components), being used. It could either be this component, or potentially a new component. Here's a few ideas:

Option 1: Make fxos-switch accept a label, like:

<fxos-switch>
  <label>My Label</label>
</fxos-switch>

Option 2: Create a new "fxos-switch-row". This could be an external component.

<fxos-switch-row>
  <label>My Label</label>
  <details>Some details here</details>
</fxos-switch-row>

Option 3: Continue to use inside of gaia, and update it to extend or include the . This could work, but I do think that we should only have one switch implementation inside of gaia, and currently almost every switch spans across the entire row.

Re-opening this to get ideas, but maybe the work will happen in a different repo. Thoughts?

@KevinGrandon KevinGrandon reopened this Dec 9, 2015
@KevinGrandon
Copy link
Member Author

After porting most switches across gaia to web components, I think it's way too much work to move away from the nested label control at this time. I think our time is better spent implementing additional web 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

No branches or pull requests

2 participants