Skip to content

Commit

Permalink
fix(input, textarea, select): use consistent sizes (#28390)
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
liamdebeasi and Ionitron authored Oct 26, 2023
1 parent dc94ae0 commit b31ecbb
Show file tree
Hide file tree
Showing 228 changed files with 63 additions and 86 deletions.
18 changes: 0 additions & 18 deletions core/src/components/input/input.ios.scss
Original file line number Diff line number Diff line change
Expand Up @@ -28,24 +28,6 @@
height: 18px;
}

// Input Wrapper
// ----------------------------------------------------------------

:host(:not(.legacy-input)) {
min-height: 44px;
}

/**
* Since the label sits on top of the element,
* the component needs to be taller otherwise the
* label will appear too close to the input text.
*/
:host(.input-label-placement-floating),
:host(.input-label-placement-stacked) {
min-height: 56px;
}


// Input - Disabled
// ----------------------------------------------------------------
// The input, label, helper text, char counter and placeholder
Expand Down
2 changes: 2 additions & 0 deletions core/src/components/input/input.md.outline.scss
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
--border-radius: 4px;
--padding-start: 16px;
--padding-end: 16px;

min-height: 56px;
}

:host(.input-fill-outline.input-shape-round) {
Expand Down
7 changes: 0 additions & 7 deletions core/src/components/input/input.md.scss
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,6 @@
letter-spacing: .0333333333em;
}

// Input Wrapper
// ----------------------------------------------------------------

:host(:not(.legacy-input)) {
min-height: 56px;
}

// Input Label
// ----------------------------------------------------------------

Expand Down
2 changes: 2 additions & 0 deletions core/src/components/input/input.md.solid.scss
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
--border-radius: 4px;
--padding-start: 16px;
--padding-end: 16px;

min-height: 56px;
}

/**
Expand Down
17 changes: 17 additions & 0 deletions core/src/components/input/input.scss
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,23 @@
--highlight-color-focused: #{current-color(base)};
}

// Input Wrapper
// ----------------------------------------------------------------

:host(:not(.legacy-input)) {
min-height: 44px;
}

/**
* Since the label sits on top of the element,
* the component needs to be taller otherwise the
* label will appear too close to the input text.
*/
:host(.input-label-placement-floating),
:host(.input-label-placement-stacked) {
min-height: 56px;
}

// Native Text Input
// --------------------------------------------------

Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
15 changes: 0 additions & 15 deletions core/src/components/select/select.ios.scss
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,6 @@
--padding-start: #{$select-ios-padding-start};
}

// TODO FW-3194 - Move this to host
:host(:not(.legacy-select)) {
min-height: 44px;
}

/**
* Since the label sits on top of the element,
* the component needs to be taller otherwise the
* label will appear too close to the select text.
*/
:host(.select-label-placement-floating),
:host(.select-label-placement-stacked) {
min-height: 56px;
}

.select-icon {
width: $select-ios-icon-size;
height: $select-ios-icon-size;
Expand Down
2 changes: 2 additions & 0 deletions core/src/components/select/select.md.outline.scss
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
--border-radius: 4px;
--padding-start: 16px;
--padding-end: 16px;

min-height: 56px;
}

:host(.select-fill-outline.select-shape-round) {
Expand Down
5 changes: 0 additions & 5 deletions core/src/components/select/select.md.scss
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,6 @@
--padding-start: #{$select-md-padding-start};
}

// TODO FW-3194 - Move this to host
:host(:not(.legacy-select)) {
min-height: 56px;
}

.select-icon {
width: $select-md-icon-size;

Expand Down
2 changes: 2 additions & 0 deletions core/src/components/select/select.md.solid.scss
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
--border-radius: 4px;
--padding-start: 16px;
--padding-end: 16px;

min-height: 56px;
}

/**
Expand Down
11 changes: 11 additions & 0 deletions core/src/components/select/select.scss
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,17 @@
// TODO FW-3194 - Move this to host
:host(:not(.legacy-select)) {
width: 100%;
min-height: 44px;
}

/**
* Since the label sits on top of the element,
* the component needs to be taller otherwise the
* label will appear too close to the select text.
*/
:host(.select-label-placement-floating),
:host(.select-label-placement-stacked) {
min-height: 56px;
}

:host(.ion-color) {
Expand Down
23 changes: 0 additions & 23 deletions core/src/components/textarea/textarea.ios.scss
Original file line number Diff line number Diff line change
Expand Up @@ -40,26 +40,3 @@
:host(.textarea-disabled) {
opacity: #{$textarea-ios-disabled-opacity};
}

// Textarea Wrapper
// ----------------------------------------------------------------

// TODO: FW-2876 - Make this style a :host style, remove :not selector
:host(:not(.legacy-textarea)) {
min-height: 44px;
}

/**
* Since the label sits on top of the element,
* the component needs to be taller otherwise the
* label will appear too close to the textarea text.
* Also, floating and stacked labels should not
* push the label down since it it
* sits on top of the textarea.
*/
:host(.textarea-label-placement-floating),
:host(.textarea-label-placement-stacked) {
--padding-top: 0px;

min-height: 56px;
}
2 changes: 2 additions & 0 deletions core/src/components/textarea/textarea.md.outline.scss
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
--border-radius: 4px;
--padding-start: 16px;
--padding-end: 16px;

min-height: 56px;
}

:host(.textarea-fill-outline.textarea-shape-round) {
Expand Down
18 changes: 0 additions & 18 deletions core/src/components/textarea/textarea.md.scss
Original file line number Diff line number Diff line change
Expand Up @@ -42,24 +42,6 @@
letter-spacing: 0.0333333333em;
}

// Textarea Wrapper
// ----------------------------------------------------------------

// TODO: FW-2876 - Make this a style on :host, remove :not selector
:host(:not(.legacy-textarea)) {
min-height: 56px;
}

/**
* Floating and stacked labels should not
* push the label down since it it
* sits on top of the textarea.
*/
:host(.textarea-label-placement-floating),
:host(.textarea-label-placement-stacked) {
--padding-top: 0px;
}

// Textarea Label
// ----------------------------------------------------------------

Expand Down
2 changes: 2 additions & 0 deletions core/src/components/textarea/textarea.md.solid.scss
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
--border-radius: 4px;
--padding-start: 16px;
--padding-end: 16px;

min-height: 56px;
}

/**
Expand Down
23 changes: 23 additions & 0 deletions core/src/components/textarea/textarea.scss
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,29 @@
box-sizing: border-box;
}

// Textarea Wrapper
// ----------------------------------------------------------------

// TODO: FW-2876 - Make this style a :host style, remove :not selector
:host(:not(.legacy-textarea)) {
min-height: 44px;
}

/**
* Since the label sits on top of the element,
* the component needs to be taller otherwise the
* label will appear too close to the textarea text.
* Also, floating and stacked labels should not
* push the label down since it it
* sits on top of the textarea.
*/
:host(.textarea-label-placement-floating),
:host(.textarea-label-placement-stacked) {
--padding-top: 0px;

min-height: 56px;
}

/**
* When the cols property is set we should
* respect that width instead of defaulting
Expand Down

0 comments on commit b31ecbb

Please sign in to comment.