fix(select): prevent floating label and placeholder from overlapping when focused - #31276
Conversation
…when focused modify placeholder visibility (opacity) css condition to show the placeholder if the select is focused *and* has no value, as opposed to only being focused closes ionic-team#30401
|
@droc101 is attempting to deploy a commit to the Ionic Team on Vercel. A member of the Team first needs to authorize it. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
thetaPC
left a comment
thetaPC
left a comment
There was a problem hiding this comment.
Please also add a snapshot test under label/select.e2e.ts that captures a focused floating select with a placeholder and no value, so we can verify this state does not regress. Applying the has-focus class directly keeps the screenshot deterministic, following the pattern in the other select e2e tests.
| */ | ||
| :host(.select-expanded.select-label-placement-floating) .native-wrapper .select-placeholder, | ||
| :host(.has-focus.select-label-placement-floating) .native-wrapper .select-placeholder, | ||
| :host(.has-focus.select-label-placement-floating):not(.has-value) .native-wrapper .select-placeholder, |
There was a problem hiding this comment.
Rather than hiding the placeholder here, let's float the label up on focus instead. This keeps it consistent with ion-input and matches the Material Design text field behavior, where focusing an empty field floats the label and shows the placeholder below.
Take a look at labelShouldFloat in select.tsx for where to handle this.
|
I'll look into adding the test now, but it might take a while as I am not familiar with writing snapshot tests. |
| await select.click(); | ||
| const cancel = page.locator('button.alert-button-role-cancel'); | ||
| await cancel.click(); |
There was a problem hiding this comment.
Focus the select's button directly here instead of opening and dismissing the overlay. It reaches the same has-focus state without the overlay animation, so the screenshot stays deterministic, and it drops the dependency on the alert markup.
| await select.click(); | |
| const cancel = page.locator('button.alert-button-role-cancel'); | |
| await cancel.click(); | |
| await page.locator('ion-select button').focus(); |
| await select.click(); | ||
| const cancel = page.locator('button.alert-button-role-cancel'); | ||
| await cancel.click(); | ||
| await expect(select).toHaveScreenshot(screenshot(`select-label-focus-floating-no-value-placeholder`)); |
There was a problem hiding this comment.
Rename this snapshot to select-label-floating-focus-no-value-placeholder so floating comes before focus, matching the other floating snapshots in this file such as select-label-floating-expanded and select-label-floating-long-text. Make sure to delete the old generated snapshots as those don't get automatically deleted when the filename changes.
| await expect(select).toHaveScreenshot(screenshot(`select-label-focus-floating-no-value-placeholder`)); | |
| await expect(select).toHaveScreenshot(screenshot(`select-label-floating-focus-no-value-placeholder`)); |
| const select = page.locator('ion-select'); | ||
| await expect(select).toHaveScreenshot(screenshot(`select-label-floating-no-value-placeholder`)); | ||
| }); | ||
| test('label should appear on top of the select when it is focused, has a placeholder, and no value', async ({ |
There was a problem hiding this comment.
This test only needs to run in MD mode and LTR direction. The label floating on focus does not vary across modes or directions, so scoping it here avoids generating 12 redundant snapshots for a single behavior. Move it into its own config block, matching the pattern in alert/test/basic/alert-tablet.e2e.ts (don't forget the comment block):
/**
* This behavior does not vary across modes/directions
*/
configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, screenshot, config }) => {
...
});
2 participants
Issue number: resolves #30401
What is the current behavior?
The floating label and placeholder on
ion-selectoverlap when focused without a valueWhat is the new behavior?
The placeholder is not shown when a floating label select is focused, as that space is taken up by the label
Does this introduce a breaking change?