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

fix(many): do not grow slotted checkboxes, radios, selects and toggles #29501

Merged
merged 7 commits into from
May 15, 2024

Conversation

brandyscarney
Copy link
Member

@brandyscarney brandyscarney commented May 14, 2024

Issue number: resolves #29423


What is the current behavior?

I fixed a bug where icon was collapsing its width when next to a checkbox, radio or toggle to match the styles of select in #29328. This caused a regression for checkboxes, radios, and toggles when slotted inside of an item. Our test coverage for this was not great, as the slotted inputs test in item had so many elements that it was not apparent that this bug was introduced. In addition, the select itself presented the same issue before my PR and this is a regression from the v7 behavior. See the following Codepens to see the regression:

What is the new behavior?

  • Updates the checkbox, radio, select, and toggle to reset the flex property when slotted.
  • Adds test coverage for the previous fix I did in fix(form-controls): adjust flex properties inside of an item #29328 where icons were collapsing their width next to checkboxes, radios and toggles. This was reproducible with a div and easier to see in a test so I used a div with a background instead of an icon.
  • Adds better test coverage for this fix which separates each component (checkbox, radio, select, toggle) into their own screenshot test to make sure the width is shrinking or expanding properly based on where it is located in an item.

Does this introduce a breaking change?

  • Yes
  • No

Other information

Before fix 9b59138 After
before fix after fix
Before regression fix After
before-regression-fix after-regression-fix

Copy link

vercel bot commented May 14, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
ionic-framework ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 14, 2024 9:32pm

@brandyscarney brandyscarney changed the title fix(item): do not grow slotted checkboxes, radios and toggles fix(many): do not grow slotted checkboxes, radios and toggles May 14, 2024
@github-actions github-actions bot added the package: core @ionic/core package label May 14, 2024
@brandyscarney brandyscarney changed the title fix(many): do not grow slotted checkboxes, radios and toggles fix(many): do not grow slotted checkboxes, radios, selects and toggles May 14, 2024
@brandyscarney brandyscarney marked this pull request as ready for review May 14, 2024 21:45
@brandyscarney brandyscarney requested a review from a team as a code owner May 14, 2024 21:45
Copy link
Contributor

@thetaPC thetaPC left a comment

Choose a reason for hiding this comment

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

LGTM

@brandyscarney brandyscarney added this pull request to the merge queue May 15, 2024
Merged via the queue into main with commit c63d07b May 15, 2024
52 checks passed
@brandyscarney brandyscarney deleted the ROU-8161 branch May 15, 2024 14:27
sean-perkins pushed a commit that referenced this pull request May 15, 2024
#29501)

Issue number: resolves #29423

---------

## What is the current behavior?
I fixed a bug where icon was collapsing its width when next to a
checkbox, radio or toggle to match the styles of select in
#29328. This caused a
regression for checkboxes, radios, and toggles when slotted inside of an
item. Our test coverage for this was not great, as the slotted inputs
test in item had so many elements that it was not apparent that this bug
was introduced. In addition, the select itself presented the same issue
before my PR and this is a regression from the v7 behavior. See the
following Codepens to see the regression:

- [Ionic v7](https://codepen.io/brandyscarney/pen/jOoPzoL)
- [Ionic v8](https://codepen.io/brandyscarney/pen/KKLpoLX)

## What is the new behavior?
- Updates the checkbox, radio, select, and toggle to reset the flex
property when slotted.
- Adds test coverage for the previous fix I did in
#29328 where icons
were collapsing their width next to checkboxes, radios and toggles. This
was reproducible with a div and easier to see in a test so I used a div
with a background instead of an icon.
- Adds better test coverage for this fix which separates each component
(checkbox, radio, select, toggle) into their own screenshot test to make
sure the width is shrinking or expanding properly based on where it is
located in an item.

## Does this introduce a breaking change?

- [ ] Yes
- [x] No

## Other information

| Before fix
9b59138
| After |
| ---| ---|
| ![before
fix](https://github.com/ionic-team/ionic-framework/assets/6577830/e27c6e3c-3d3a-4889-a44b-5f4a9a6ba552)
| ![after
fix](https://github.com/ionic-team/ionic-framework/assets/6577830/a05829d6-f776-4f0d-b7eb-cb8177449c90)
|

<table width="100%">
  <tr align="center">
    <td width="50%"><b>Before regression fix</b></td>
    <td width="50%"><b>After</b></td>
  </tr>
  <tr>
<td width="50%"><img alt="before-regression-fix"
src="https://github.com/ionic-team/ionic-framework/assets/6577830/bb1aea84-6c83-4fbe-96ad-855c1c9cca95"></td>
<td width="50%"><img alt="after-regression-fix"
src="https://github.com/ionic-team/ionic-framework/assets/6577830/655dab88-55a9-4961-a7fb-2a3233aa0004"></td>
  </tr>
</table>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: core @ionic/core package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: ion-item + ion-checkbox on 'start' slot not respect width on v8 (v7 was ok)
2 participants