fix(select): support floating labels with slotted content - #31326
fix(select): support floating labels with slotted content#31326brandyscarney wants to merge 32 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
I renamed these screenshots from select-slots to select-slot to match the folder name.
|
|
||
| configs().forEach(({ title, screenshot, config }) => { | ||
| test.describe(title('select: start and end slots (visual checks)'), () => { | ||
| test.describe(title('select: slot'), () => { |
There was a problem hiding this comment.
This was updated to match the folder name, following how we title other tests.
| test('should not have visual regressions with a floating label when expanded', async ({ page }) => { | ||
| test.info().annotations.push({ | ||
| type: 'issue', | ||
| description: 'https://github.com/ionic-team/ionic-framework/issues/30402', |
There was a problem hiding this comment.
I noticed this bug and fixed it while I was cleaning up the styles so I added a test for it here.
There was a problem hiding this comment.
After updating all of the screenshots I found that this is technically covered by this one: https://github.com/ionic-team/ionic-framework/pull/31326/changes?#diff-5a03488c7650116b1b96323c5b1ec6fb3d652e44fd7c78d9a162a22076fcafe2
I could remove this test if desired and add the issue number on that test.
| test('should not have visual regressions with a floating label when expanded', async ({ page }) => { | ||
| test.info().annotations.push({ | ||
| type: 'issue', | ||
| description: 'https://github.com/ionic-team/ionic-framework/issues/30402', |
There was a problem hiding this comment.
I noticed this bug and fixed it while I was cleaning up the styles so I added a test for it here.
| } | ||
|
|
||
| /* Hide the backdrop for action sheets */ | ||
| ion-action-sheet.select-action-sheet ion-backdrop { |
There was a problem hiding this comment.
I changed all of the selects here to use an action sheet with a clear backdrop so that you could see the states better when interacting with them.
ShaneK
left a comment
There was a problem hiding this comment.
Looking really great! I noticed a few issues and a few nits, hopefully I explained things well enough that it's not too bad to fix up!
| const target = ev.target as HTMLElement; | ||
| const nativeWrapper = this.el.shadowRoot?.querySelector('.native-wrapper'); | ||
|
|
||
| if (!nativeWrapper?.contains(target)) { |
There was a problem hiding this comment.
Clicking a slotted ion-button still fires its own handler, but a listener on document gets nothing. On major-9.0 both fire once.
Slotted nodes are never inside .native-wrapper, so they always take the stopPropagation() branch. That's the case onClick avoids stopping, so React's onClick on slotted content stops working.
| await expect(select).not.toHaveClass(/select-expanded/); | ||
| await expect(select).toHaveScreenshot(screenshot(`select-slot-label-floating-value`)); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Could should not open select when slotted buttons are clicked come back? It's the only coverage for onLabelClick, and this PR rewrites that handler.
| const isRTL = document.dir === 'rtl'; | ||
| const sign = isRTL ? '' : '-'; |
There was a problem hiding this comment.
| const isRTL = document.dir === 'rtl'; | |
| const sign = isRTL ? '' : '-'; | |
| const sign = isRTL(this.el) ? '' : '-'; |
Using document.dir here misses a dir set on the host. With dir="rtl" on the select in an LTR page it renders RTL but the offset comes out -48px instead of 48px, so the label lands about 96px off its notch.
| return ''; | ||
| } | ||
|
|
||
| const startSlotWidth = startSlot.getBoundingClientRect().width; |
There was a problem hiding this comment.
The offset only refreshes when the slot mutation fires, so a start slot that changes size without a DOM change keeps the old value. Widening slotted content with CSS leaves the real width at 176px and the offset still at -36.4px, which leaves the label mid-field.
A ResizeObserver on .select-start would cover it. Might be worth a short doc comment on this method too, it's doing a bit more than the name suggests.
| */ | ||
| this.skipLabelTransition = true; | ||
|
|
||
| requestAnimationFrame(() => { |
There was a problem hiding this comment.
This isn't cancelled on disconnect, so it can set state after the component is gone. Storing the handle and clearing it next to the slot controller would fix it.
|
|
||
| configs().forEach(({ title, screenshot, config }) => { | ||
| test.describe(title('select: start and end slots (visual checks)'), () => { | ||
| test.describe(title('select: slot'), () => { |
There was a problem hiding this comment.
The new screenshots cover start and floating but not stacked, which goes through the same restructure. Worth adding here, or a follow-up card if you'd rather keep this one tight.
Co-authored-by: Shane <shane.king@outsystems.com>
Co-authored-by: Shane <shane.king@outsystems.com>
Issue number: resolves #30402
What is the current behavior?
Selects with a floating label and a start or end slot always display the label in the floated state, regardless of whether the select contains a value:
What is the new behavior?
--placeholder-opacityinstead of1, matching the other select label placements.mdspecification.Does this introduce a breaking change?
Internal DOM Structure Changes
The component's internal DOM structure has been restructured to support floating labels with slotted start and end content. Additionally, the structure of the component has been reorganized, with some elements now grouped differently than before. The
innerwrapper element has been removed, and its content has been split across separate wrapper elements for the start slot, control, and end slot. This may introduce breaking changes for developers who rely on the component's internal DOM structure or apply custom styling to internal elements.Developers who previously styled
ion-select::part(inner)should migrate to targeting the updated component structure using the following CSS parts instead:ion-select::part(start)- Target the start slot wrapperion-select::part(control)- Target the control wrapper containing the label and native select. When the label is not floating or stacked, this part also contains the dropdown icon.ion-select::part(end)- Target the end slot wrapper. When the label is floating or stacked, this part also contains the dropdown icon.Other information
Preview