Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
brandyscarney
left a comment
There was a problem hiding this comment.
I will leave more info on the ticket!
| await expect(page.locator('ion-select')).not.toHaveClass(/has-focus/); | ||
| }); | ||
|
|
||
| test('should activate slotted form controls', async ({ page }) => { |
There was a problem hiding this comment.
We should verify that activating a form control doesn't open the select.
| test('should emit one click when a slotted icon is clicked', async ({ page }) => { | ||
| const clickEvent = await page.spyOnEvent('click'); | ||
|
|
||
| await page.locator('#start-icon').click(); | ||
|
|
||
| expect(clickEvent).toHaveReceivedEventTimes(1); | ||
|
|
||
| const event = clickEvent.events[0]; | ||
| expect((event.target as HTMLElement).tagName.toLowerCase()).toBe('ion-icon'); | ||
| }); | ||
|
|
||
| /** | ||
| * Decorative slotted content behaves the same as clicking the select | ||
| * itself, so it opens the overlay. | ||
| */ | ||
| test('should open when a slotted icon is clicked', async ({ page }) => { | ||
| const ionAlertDidPresent = await page.spyOnEvent('ionAlertDidPresent'); | ||
|
|
||
| await page.locator('#start-icon').click(); | ||
| await ionAlertDidPresent.next(); | ||
|
|
||
| await expect(page.locator('ion-alert')).toBeVisible(); | ||
| }); |
There was a problem hiding this comment.
Can we combine these?
| test('should emit one click when a slotted icon is clicked', async ({ page }) => { | |
| const clickEvent = await page.spyOnEvent('click'); | |
| await page.locator('#start-icon').click(); | |
| expect(clickEvent).toHaveReceivedEventTimes(1); | |
| const event = clickEvent.events[0]; | |
| expect((event.target as HTMLElement).tagName.toLowerCase()).toBe('ion-icon'); | |
| }); | |
| /** | |
| * Decorative slotted content behaves the same as clicking the select | |
| * itself, so it opens the overlay. | |
| */ | |
| test('should open when a slotted icon is clicked', async ({ page }) => { | |
| const ionAlertDidPresent = await page.spyOnEvent('ionAlertDidPresent'); | |
| await page.locator('#start-icon').click(); | |
| await ionAlertDidPresent.next(); | |
| await expect(page.locator('ion-alert')).toBeVisible(); | |
| }); | |
| /** | |
| * Decorative slotted content behaves the same as clicking the select | |
| * itself, so it opens the overlay. | |
| */ | |
| test('should emit one click and open the select when a slotted icon is clicked', async ({ page }) => { | |
| const clickEvent = await page.spyOnEvent('click'); | |
| await page.locator('#start-icon').click(); | |
| expect(clickEvent).toHaveReceivedEventTimes(1); | |
| const event = clickEvent.events[0]; | |
| expect((event.target as HTMLElement).tagName.toLowerCase()).toBe('ion-icon'); | |
| await ionAlertDidPresent.next(); | |
| await expect(page.locator('ion-alert')).toBeVisible(); | |
| }); |
| test('should emit one click when a slotted button is clicked', async ({ page }) => { | ||
| const clickEvent = await page.spyOnEvent('click'); | ||
|
|
||
| await page.locator('#end-button').click(); | ||
|
|
||
| expect(clickEvent).toHaveReceivedEventTimes(1); | ||
| }); | ||
|
|
||
| test('should not open when a slotted button is clicked', async ({ page }) => { | ||
| await page.locator('#end-button').click(); | ||
|
|
||
| await expect(page.locator('ion-alert')).toHaveCount(0); | ||
| }); | ||
|
|
||
| test('should not focus the select when a slotted button is clicked', async ({ page }) => { | ||
| await page.locator('#end-button').click(); | ||
|
|
||
| await expect(page.locator('ion-select')).not.toHaveClass(/has-focus/); | ||
| }); |
There was a problem hiding this comment.
Is this the best name for this? It doesn't only handle slotted clicks, it handles all clicks including the native input. What about renaming the file to click-controller (or similar) to match the other files in this directory:
notch-controller.tsstart-container-controller.ts
| /** | ||
| * Whether a click started on content slotted into a form control's start or | ||
| * end slot. | ||
| * | ||
| * The slotted element is compared against the host in case the form control | ||
| * itself is slotted into, for example, an item. Without that check a control | ||
| * carrying slot="start"/"end" would treat every click on itself as a slotted | ||
| * click. | ||
| */ | ||
| export const isSlottedClick = (ev: Event, el: HTMLElement): boolean => { | ||
| const slotted = (ev.target as HTMLElement).closest('[slot="start"], [slot="end"]'); | ||
|
|
||
| return slotted !== null && slotted !== el && el.contains(slotted); | ||
| }; |
There was a problem hiding this comment.
If you split this to have getSlottedClickContent, select can be updated to call it (see select.tsx comment)
| /** | |
| * Whether a click started on content slotted into a form control's start or | |
| * end slot. | |
| * | |
| * The slotted element is compared against the host in case the form control | |
| * itself is slotted into, for example, an item. Without that check a control | |
| * carrying slot="start"/"end" would treat every click on itself as a slotted | |
| * click. | |
| */ | |
| export const isSlottedClick = (ev: Event, el: HTMLElement): boolean => { | |
| const slotted = (ev.target as HTMLElement).closest('[slot="start"], [slot="end"]'); | |
| return slotted !== null && slotted !== el && el.contains(slotted); | |
| }; | |
| /** | |
| * The content slotted into a form control's start or end slot that a click | |
| * started on, or `null` when the click did not start on slotted content. | |
| * | |
| * The slotted element is compared against the host in case the form control | |
| * itself is slotted into, for example, an item. Without that check a control | |
| * carrying slot="start"/"end" would treat every click on itself as a slotted | |
| * click. | |
| */ | |
| export const getSlottedClickContent = (ev: Event, el: HTMLElement): HTMLElement | null => { | |
| const slotted = (ev.target as HTMLElement).closest<HTMLElement>('[slot="start"], [slot="end"]'); | |
| return slotted !== null && slotted !== el && el.contains(slotted) ? slotted : null; | |
| }; | |
| /** | |
| * Whether a click started on content slotted into a form control's start or | |
| * end slot. | |
| */ | |
| export const isSlottedClick = (ev: Event, el: HTMLElement): boolean => getSlottedClickContent(ev, el) !== null; |
| const deepTarget = ev.composedPath()[0] as HTMLElement; | ||
| if (isSlottedClick(ev, this.el) && deepTarget.closest(INTERACTIVE_SLOTTED_CONTENT) !== null) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Changing to the following fixes the issue where the select opens from a slotted checkbox/radio/toggle:
| const deepTarget = ev.composedPath()[0] as HTMLElement; | |
| if (isSlottedClick(ev, this.el) && deepTarget.closest(INTERACTIVE_SLOTTED_CONTENT) !== null) { | |
| return; | |
| } | |
| const slotted = getSlottedClickContent(ev, this.el); | |
| if (slotted !== null) { | |
| const interactive = (ev.target as HTMLElement).closest(INTERACTIVE_SLOTTED_CONTENT); | |
| if (interactive !== null && slotted.contains(interactive)) { | |
| return; | |
| } | |
| } |
| if (this.hasSlottedClick) { | ||
| ev.stopPropagation(); | ||
| this.hasSlottedClick = false; | ||
| } |
There was a problem hiding this comment.
A lot of this is duplicated from the slotted click controller - can this not be combined with that?
Issue number: resolves internal
What is the current behavior?
Clicking content in the
startorendslot behaves differently in each form control.ion-inputandion-textareaemit the click event twice, whileion-selectemits once but does not respond to the click at all.The duplicate comes from the browser rather than from Ionic emitting twice. The wrapping
<label>forwards the click to the native control, and that forwarded click is re-emitted from the host. Browsers skip this forwarding when the click lands on interactive content, which is why slotted buttons were never affected.ion-selectseparately cancels the default action on every slotted click. A slotted link does not navigate and a slotted checkbox does not toggle, while the same markup works inion-input.What is the new behavior?
ion-inputandion-textareafocus,ion-selectopens.ion-select.Does this introduce a breaking change?
Other information
Previews: