Skip to content

Commit

Permalink
fix(select): text does not overlap icon (#27125)
Browse files Browse the repository at this point in the history
<!-- Please refer to our contributing documentation for any questions on
submitting a pull request, or let us know here if you need any help:
https://ionicframework.com/docs/building/contributing -->

<!-- Some docs updates need to be made in the `ionic-docs` repo, in a
separate PR. See
https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#modifying-documentation
for details. -->

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


<!-- Issues are required for both bug fixes and features. -->
Issue URL: resolves #27081

When using stacked/floating labels the icon needs to be centered with
the entire component, not just the placeholder/selected text. As a
result, we set `position: absolute`. However, this causes the long
selected texts to overlap the icon. This is not happening with
non-stacked/floating labels because the icon is `position: relative` and
follows the normal document flow.

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

- Moved the icon sizes to sass variables
- Added code to set the .native-wrapper width to 100% minus the width of
the icon _and_ the additional margin that .select-icon adds

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

---------

Co-authored-by: ionitron <[email protected]>
Co-authored-by: Brandy Carney <[email protected]>
  • Loading branch information
3 people committed Apr 17, 2023
1 parent 063cd9f commit f6ff206
Show file tree
Hide file tree
Showing 55 changed files with 35 additions and 10 deletions.
12 changes: 10 additions & 2 deletions core/src/components/select/select.ios.scss
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,18 @@
}

.select-icon {
width: 18px;
height: 18px;
width: $select-ios-icon-size;
height: $select-ios-icon-size;

// Color deviates from the iOS spec, but satisfies WCAG AAA:
// https://www.w3.org/TR/WCAG20-TECHS/G18.html
color: #{$text-color-step-350};
}

// Select Native Wrapper
// ----------------------------------------------------------------

:host(.select-label-placement-stacked) .native-wrapper,
:host(.select-label-placement-floating) .native-wrapper {
width: calc(100% - $select-ios-icon-size - $select-icon-margin-start);
}
3 changes: 3 additions & 0 deletions core/src/components/select/select.ios.vars.scss
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ $select-ios-padding-start: $item-ios-padding-start !default;
/// @prop - Color of the select icon
$select-ios-icon-color: $text-color-step-650 !default;

/// @prop - Size of the select icon
$select-ios-icon-size: 18px !default;

/// @prop - Color of the select placeholder
$select-ios-placeholder-color: $select-ios-icon-color !default;

Expand Down
10 changes: 9 additions & 1 deletion core/src/components/select/select.md.scss
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
}

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

transition: transform .15s cubic-bezier(.4, 0, .2, 1);

Expand Down Expand Up @@ -142,3 +142,11 @@
:host(.select-shape-round) {
--border-radius: 16px;
}

// Select Native Wrapper
// ----------------------------------------------------------------

:host(.select-label-placement-stacked) .native-wrapper,
:host(.select-label-placement-floating) .native-wrapper {
width: calc(100% - $select-md-icon-size - $select-icon-margin-start);
}
3 changes: 3 additions & 0 deletions core/src/components/select/select.md.vars.scss
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ $select-md-padding-start: $item-md-padding-start !default;
/// @prop - Color of the select icon
$select-md-icon-color: $text-color-step-600 !default;

/// @prop - Size of the select icon
$select-md-icon-size: 13px !default;

/// @prop - Color of the select placeholder
$select-md-placeholder-color: $select-md-icon-color !default;

Expand Down
2 changes: 1 addition & 1 deletion core/src/components/select/select.scss
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ button {
// --------------------------------------------------

.select-icon {
@include margin(0, 0, 0, 4px);
@include margin(0, 0, 0, $select-icon-margin-start);

position: relative;
}
Expand Down
3 changes: 3 additions & 0 deletions core/src/components/select/select.vars.scss
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,6 @@

/// @prop - How much to scale the floating label by
$select-floating-label-scale: 0.75 !default;

/// @prop - Margin start of the select icon
$select-icon-margin-start: 4px !default;
12 changes: 6 additions & 6 deletions core/src/components/select/test/label/select.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,17 +168,17 @@ test.describe('select: label', () => {
`select-label-floating-expanded-${page.getSnapshotSettings()}.png`
);
});
test('long label should truncate', async ({ page }) => {
test('long text should truncate', async ({ page }) => {
await page.setContent(`
<ion-select label="Label Label Label Label Label Label Label Label Label Label Label Label Label Label Label" label-placement="floating" value="apples" placeholder="Select a Fruit">
<ion-select-option value="apples">Apples</ion-select-option>
<ion-select-option value="apples">Apples Apples Apples Apples Apples Apples Apples Apples</ion-select-option>
</ion-select>
`);

const select = page.locator('ion-select');

expect(await select.screenshot({ animations: 'disabled' })).toMatchSnapshot(
`select-label-floating-long-label-${page.getSnapshotSettings()}.png`
`select-label-floating-long-text-${page.getSnapshotSettings()}.png`
);
});
});
Expand Down Expand Up @@ -219,17 +219,17 @@ test.describe('select: label', () => {
`select-label-stacked-expanded-${page.getSnapshotSettings()}.png`
);
});
test('long label should truncate', async ({ page }) => {
test('long text should truncate', async ({ page }) => {
await page.setContent(`
<ion-select label="Label Label Label Label Label Label Label Label Label Label Label Label Label Label Label" label-placement="stacked" value="apples" placeholder="Select a Fruit">
<ion-select-option value="apples">Apples</ion-select-option>
<ion-select-option value="apples">Apples Apples Apples Apples Apples Apples Apples Apples</ion-select-option>
</ion-select>
`);

const select = page.locator('ion-select');

expect(await select.screenshot({ animations: 'disabled' })).toMatchSnapshot(
`select-label-stacked-long-label-${page.getSnapshotSettings()}.png`
`select-label-stacked-long-text-${page.getSnapshotSettings()}.png`
);
});
});
Expand Down
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
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.
Binary file not shown.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.

0 comments on commit f6ff206

Please sign in to comment.