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.

Sign up for GitHub

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

Jump to bottom

bug: MD inconsistency: ion-item height is different with some children #28388

Closed
3 tasks done
cmaas opened this issue Oct 20, 2023 · 4 comments · Fixed by #28390
Closed
3 tasks done

bug: MD inconsistency: ion-item height is different with some children #28388

cmaas opened this issue Oct 20, 2023 · 4 comments · Fixed by #28390
Labels
package: core @ionic/core package type: bug a confirmed bug report

Comments

@cmaas
Copy link

cmaas commented Oct 20, 2023

Prerequisites

Ionic Framework Version

v7.x

Current Behavior

In v7, some elements have a new mode that get their labels as props such as: ion-input, ion-select. This "non-legacy" mode sets their min-height to 56px (why though?), see here:

ionic-framework/core/src/components/select/select.md.scss

Line 23 in c801e2a

:host(:not(.legacy-select)) {

However, other elements such as ion-toggle or ion-label (used for navigation in an ion-item) do not have a label prop and hence, don't have this rule applied. This results in inconsistent vertical spacing. See the demo linked below.

Expected Behavior

  1. Make spacing consistent
  2. Revert (or explain) the choice of setting a, imho, rather unpleasant min-height of 56px.

Steps to Reproduce

See demo

Code Reproduction URL

https://flems.io/#0=N4IgzgpgNhDGAuEAmIBcIB0ALeBbKIANCAGYCWMYaA2qAHYCGuEamO+RIsA9nYn6wA87KAD4AOnQAEU4RAZIJ0mbPhl4MUQEleZWFIAqEMPEEB6NRohKVssLABOZAA7wp8AJ7OIAXnEhcbiQAVxh-KTAHWD8QHHhnMFQzM1gkOgwAKzAkaDIANwcMOgh4MzpnXDMAATJdWBTuBwgzJDITM1q6PQ66jGNcTLB-UXN7J1cbFUEoMjoAaykmqBiTD0osCBLwrCaSGLiEpJS0wZyZgqKSsorqzu6eJpSwMB6u2AwAI2C6JBgMWGe4TMk3MGwUSkEHyCHhBnQAtGCcg5JrZBPD4NxuFAPgwHFIeFBGjFnE5cLiPMNJLZqbJ0epNAAJbjMcx0qwoqavOEYrE45FU2RcxEQfnKNG8OE8PgQPj4qAMZ4xeHOBStOgAc0pylRzlEAHVoDxmO5uFIAMr8WAUKQAQWcznN8FxiEK5l1AtR8JmJg5NPFdDh6gguF9NKm8NmzmCbnlH2gMQAmtxgnjGMaABQAVgAbM4AB4ASnCnm8MUQefgw1ZEsj0dDqK5QZDHrD-sDiGb2rDnpr5WjUlj8f8SZTUjTECkWdzheLXl8-nLlZAI0bffg9c5EY7G5kbabI3R3HV6s0BiPJ4n8CwbUnABYABz5gvVgMY4+aF-t4M72k1jtSHInQoMsHGCCBtl2GIzBJbhyBgV49C1bs-S9Bg4zEAAFBxYIoCcMIYdUJ3TB8n0-QcxBbP1G23SjqT3Giu27NtIBgBABzQocQAAEQgEgGFCGMOKgSccyfcJnHlWAICwLEkRiAAxBg8kaIMpHk0D1CQ5CewDFi4HgOFuFcTopDyBgoDAmIGHtMJlztCSIE-PSEEM4zeB-HS4WcgyjLUXhTPMyz-BxRhGGGAAhBhQoYJzoH01y-LoDzwwlbyEpMsyLPnEBGiiwjhgAeQcPLHK5NLfM6ZLYtY9daIbLdv20mRaLItparFLkpX4drzChJAYUkUE8AouhJE4bzOiodAACZs1QTMAAYQAAX0IegmBYdBBk4LqZXgVgVrWkBx1Yf5nh23huoO5aAF1lqAA

Ionic Info

CDN bundle current version

Additional Information

No response
@ionitron-bot ionitron-bot bot added the triage label Oct 20, 2023
@liamdebeasi liamdebeasi self-assigned this Oct 20, 2023
@liamdebeasi liamdebeasi added package: core @ionic/core package type: bug a confirmed bug report labels Oct 20, 2023
@liamdebeasi liamdebeasi mentioned this issue Oct 20, 2023
Merged
2 tasks
@liamdebeasi
Copy link
Contributor

liamdebeasi commented Oct 20, 2023 •
edited
Loading

Thanks! Can you give this dev build a try? 7.5.2-dev.11697818830.1a33c881

I think the problem was we originally added min-height: 56px to align with the Material Design spec for the outlined/solid inputs. However, we accidentally applied it in the general MD stylesheet rather than the stylesheets specifically for the outline/solid inputs causing non-filled inputs to have this min-height too.

@liamdebeasi liamdebeasi removed their assignment Oct 20, 2023
@cmaas
Copy link
Author

cmaas commented Oct 20, 2023

Thanks! Can you give this dev build a try? 7.5.2-dev.11697818830.1a33c881

Looking good, thanks! Demo

github-merge-queue bot pushed a commit that referenced this issue Oct 26, 2023
@liamdebeasi @Ionitron
fix(input, textarea, select): use consistent sizes (#28390)
b31ecbb 
aIssue number: resolves #28388

---------

<!-- 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. -->

We added a `min-height: 56px` to the input, textarea, and select
components for MD mode. However, these were added for the outline/solid
style inputs to align with the Material Design spec:
https://material-components.github.io/material-components-web-catalog/#/component/text-field

They should not apply to regular inputs in an item. The end result is
inconsistently sized items when used with non-control items.

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

- Non-filled and non-stacked/floating label controls are now have a
minimum height of 44px.

There should be **no changes** to the following types of controls:

1. iOS controls (all variants)
2. MD filled controls
3. MD stacked controls

## 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. -->

Dev build: `7.5.2-dev.11697818830.1a33c881`

---------

Co-authored-by: ionitron <[email protected]>
@liamdebeasi
Copy link
Contributor

Thanks for the issue. This has been resolved via #28390, and a fix will be available in an upcoming release of Ionic Framework.

@ionitron-bot Ionitron Bot
Copy link

ionitron-bot bot commented Nov 25, 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 Nov 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
package: core @ionic/core package type: bug a confirmed bug report
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(input, textarea, select): use consistent sizes ionic-team/ionic-framework
2 participants
@liamdebeasi @cmaas