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

feat: checkbox, radio, range, toggle expose label shadow part #28300

Closed
3 tasks done
muuvmuuv opened this issue Oct 7, 2023 · 12 comments
Closed
3 tasks done

feat: checkbox, radio, range, toggle expose label shadow part #28300

muuvmuuv opened this issue Oct 7, 2023 · 12 comments
Labels
package: core @ionic/core package type: feature request a new feature, enhancement, or improvement

Comments

@muuvmuuv
Copy link

muuvmuuv commented Oct 7, 2023

Prerequisites

Describe the Feature Request

As for all other input elements like ion-select and ion-input, ion-range should also get the new implementation of labels with a part attribute attached for styling and customizations.

image

Describe the Use Case

I am building a consistent styling for all input elements in our app where the label is outside the input box, but the input box has a shadow and other styling. This works for all other elements pretty well, thanks for the work in that.

image

Describe Preferred Solution

Slots

Describe Alternatives

No shadow-dom

Related Code

No response

Additional Information

I remember that there have been a lot of label issues opened, but couldn't find one related to slots and ion-range.

@ionitron-bot ionitron-bot bot added the triage label Oct 7, 2023
@liamdebeasi liamdebeasi self-assigned this Oct 9, 2023
@liamdebeasi
Copy link
Contributor

Hey there,

I'm not sure I fully understand this feature request. Range already has a label slot: https://ionicframework.com/docs/api/range#slots

@liamdebeasi liamdebeasi added the needs: reply the issue needs a response from the user label Oct 9, 2023
@liamdebeasi liamdebeasi removed their assignment Oct 9, 2023
@ionitron-bot ionitron-bot bot removed the triage label Oct 9, 2023
@muuvmuuv
Copy link
Author

muuvmuuv commented Oct 9, 2023

Ah yes, sorry, it's not clear because it is both named “slot”. I mean for styling, that I have a slot in HTML to select via CSS when it is in the shadow-dom. Alternatively, avoid shadow-dom like it's in some other ion components when using labels. I think it is really inconsistent between components.

@ionitron-bot ionitron-bot bot added triage and removed needs: reply the issue needs a response from the user labels Oct 9, 2023
@liamdebeasi
Copy link
Contributor

liamdebeasi commented Oct 9, 2023

When using the label slot all of the content is in the light DOM, so you can style it directly: https://codepen.io/liamdebeasi/pen/qBLgdpR

Using the label slot is our recommendation for when you need to use custom HTML/CSS. The label property is great for plaintext use cases, though not so much for the use case you provided. This is why we provide both options.

edit:

Also worth noting that you can use the same label slot approach I provided above for inputs and selects too if you want to reduce code duplication.

@liamdebeasi liamdebeasi added the needs: reply the issue needs a response from the user label Oct 9, 2023
@ionitron-bot ionitron-bot bot removed the triage label Oct 9, 2023
@muuvmuuv
Copy link
Author

muuvmuuv commented Oct 9, 2023

Liam I am so sorry, I meant "part"... hard week, pretty under stress with migrating our forms module and implementing other features.

Here is my repl. You see that range has shadow-dom without [part], ion-input has no shadow-dom and ion-select has shadow-dom but with [part]. That is what I meant with inconsistency.

https://codepen.io/muuvmuuv-the-selector/pen/ExGrjRg

@ionitron-bot ionitron-bot bot added triage and removed needs: reply the issue needs a response from the user labels Oct 9, 2023
@muuvmuuv muuvmuuv changed the title feat: ion-range label slots feat: ion-range label part attribute missing Oct 9, 2023
@liamdebeasi
Copy link
Contributor

Thanks for the clarification. So the reason why there appears to be styling inconsistency is two reasons:

  1. You're using different APIs meant for different use cases.

The "label" shadow part on ion-select is designed for use cases where you want to render the label and the container separately. For example, developers may wish to have a border around the container and have the label render outside of that container. We don't provide a "label" shadow part on the range because that component does not have the same use case as the select.

The "label" slot is for developers who wish to only customize the label. We provide slotted labels all of our form controls so you can style the labels in a consistent manner.

  1. Input does not use the shadow DOM due to an old WebKit bug.

We do have plans to consolidate our form controls to use the Shadow DOM, but we need to wait for drop support for older versions of iOS unfortunately.

@liamdebeasi liamdebeasi added the needs: reply the issue needs a response from the user label Oct 9, 2023
@ionitron-bot ionitron-bot bot removed the triage label Oct 9, 2023
@muuvmuuv
Copy link
Author

muuvmuuv commented Oct 9, 2023

Thanks, so then it makes sense ion-input differs.

For example, developers may wish to have a border around the container and have the label render outside of that container. We don't provide a "label" shadow part on the range because that component does not have the same use case as the select.

This is exactly what I want to acomplish (see my screenshot) but not possible at the moment due to no access to the inner shadow-dom with the part attribute.

I don't really understand how the both cases differ for label. The label slot becomes the shadow part on ion-select and I need to use the slot because the label is coming from our backend with HTML.

As you see in my link above (updated it with styling), I have no problems styling the other controls, but the ion-range has no part's attributes I could select.

@ionitron-bot ionitron-bot bot added triage and removed needs: reply the issue needs a response from the user labels Oct 9, 2023
@liamdebeasi
Copy link
Contributor

liamdebeasi commented Oct 23, 2023

Thanks for the clarification. I think it makes sense to just expose the shadow part for the components since it's confusing that some form controls have it and some do not. When implementing this, I think we should expose the shadow part for range, toggle, checkbox, and radio too (select already has it as you noted).

@liamdebeasi liamdebeasi changed the title feat: ion-range label part attribute missing feat: checkbox, radio, range, toggle expose label shadow part Oct 23, 2023
@liamdebeasi liamdebeasi added type: feature request a new feature, enhancement, or improvement package: core @ionic/core package labels Oct 23, 2023
@ionitron-bot ionitron-bot bot removed the triage label Oct 23, 2023
@AntoscencoVladimir

This comment was marked as spam.

@liamdebeasi
Copy link
Contributor

Please do not tag the team asking for updates. This issue was only triaged earlier this week, so we have not had the chance to work on it yet.

Someone from the team will comment here when this feature has been implemented.

@Deval99
Copy link

Deval99 commented Nov 7, 2023

I am using ion-select, and i want to customise the label
it uses ion-popover instead of ion-select-option, as i am using interface="popover"
slot attribute adds a label inside ion-select-option, not in ion-popover

main problem is, label is inside ion-radio,
image
unlike previous version
image

mapsandapps added a commit that referenced this issue Dec 1, 2023
Issue number: Part of #28300 

---------

<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->

<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->

## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->
Developers are unable to adjust margin, width, etc. of the checkbox
label

## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->

- Checkbox label has a shadow part.

## Does this introduce a breaking change?

- [ ] Yes
- [x] No

<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->


## Other information

<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
As part of this work, I investigated moving `pointer-events: none` up
the DOM tree so developers wouldn't be able to override it with the
shadow part. In my testing, I was unable to see any difference in
behavior with vs without `pointer-events: none`. Therefore, I removed it
entirely.
mapsandapps added a commit that referenced this issue Dec 1, 2023
Issue number: Part of #28300 

---------

<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->

<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->

## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->
Developers are unable to adjust margin, width, etc. of the radio label

## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->

- Radio label has a shadow part.

## Does this introduce a breaking change?

- [ ] Yes
- [x] No

<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->


## Other information

<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->
As part of this work, I investigated moving `pointer-events: none` up
the DOM tree so developers wouldn't be able to override it with the
shadow part. In my testing, I was unable to see any difference in
behavior with vs without `pointer-events: none`. Therefore, I removed it
entirely.
@liamdebeasi
Copy link
Contributor

Hi everyone,

This feature has been implemented in checkbox, radio, range, and toggle and will be available in an upcoming minor release of Ionic. Thanks!

Copy link

ionitron-bot bot commented Dec 31, 2023

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Dec 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
package: core @ionic/core package type: feature request a new feature, enhancement, or improvement
Projects
None yet
Development

No branches or pull requests

4 participants