From 378c63264366964e6ea11e1a2ff8791a40f182d4 Mon Sep 17 00:00:00 2001 From: Sean Perkins Date: Tue, 25 Jan 2022 16:42:57 -0500 Subject: [PATCH] fix(datetime): timepicker popover will position relative to click target (#24616) Resolves #24531, #24415 Co-authored-by: mixalbl4 > --- core/api.txt | 2 +- core/src/components.d.ts | 3 +- core/src/components/datetime/datetime.tsx | 21 +++++++- .../components/datetime/test/position/e2e.ts | 27 ++++++++++ .../datetime/test/position/index.html | 51 +++++++++++++++++++ .../picker-column-internal.tsx | 6 ++- core/src/components/popover/popover.tsx | 2 +- core/src/components/popover/readme.md | 2 +- 8 files changed, 106 insertions(+), 8 deletions(-) create mode 100644 core/src/components/datetime/test/position/e2e.ts create mode 100644 core/src/components/datetime/test/position/index.html diff --git a/core/api.txt b/core/api.txt index ce9578268f..2cd7de4b6a 100644 --- a/core/api.txt +++ b/core/api.txt @@ -901,7 +901,7 @@ ion-popover,prop,triggerAction,"click" | "context-menu" | "hover",'click',false, ion-popover,method,dismiss,dismiss(data?: any, role?: string | undefined, dismissParentPopover?: boolean) => Promise ion-popover,method,onDidDismiss,onDidDismiss() => Promise> ion-popover,method,onWillDismiss,onWillDismiss() => Promise> -ion-popover,method,present,present(event?: MouseEvent | TouchEvent | PointerEvent | undefined) => Promise +ion-popover,method,present,present(event?: MouseEvent | TouchEvent | PointerEvent | CustomEvent | undefined) => Promise ion-popover,event,didDismiss,OverlayEventDetail,true ion-popover,event,didPresent,void,true ion-popover,event,ionPopoverDidDismiss,OverlayEventDetail,true diff --git a/core/src/components.d.ts b/core/src/components.d.ts index 8c6b829c21..69025e5fb7 100644 --- a/core/src/components.d.ts +++ b/core/src/components.d.ts @@ -1823,6 +1823,7 @@ export namespace Components { * If `true`, tapping the picker will reveal a number input keyboard that lets the user type in values for each picker column. This is useful when working with time pickers. */ "numericInput": boolean; + "scrollActiveItemIntoView": () => Promise; /** * The selected option in the picker. */ @@ -1918,7 +1919,7 @@ export namespace Components { /** * Present the popover overlay after it has been created. Developers can pass a mouse, touch, or pointer event to position the popover relative to where that event was dispatched. */ - "present": (event?: MouseEvent | TouchEvent | PointerEvent | undefined) => Promise; + "present": (event?: MouseEvent | TouchEvent | PointerEvent | CustomEvent | undefined) => Promise; /** * When opening a popover from a trigger, we should not be modifying the `event` prop from inside the component. Additionally, when pressing the "Right" arrow key, we need to shift focus to the first descendant in the newly presented popover. */ diff --git a/core/src/components/datetime/datetime.tsx b/core/src/components/datetime/datetime.tsx index 0865b9546c..8d63783d16 100644 --- a/core/src/components/datetime/datetime.tsx +++ b/core/src/components/datetime/datetime.tsx @@ -1345,9 +1345,13 @@ export class Datetime implements ComponentInterface { const { popoverRef } = this; if (popoverRef) { - this.isTimePopoverOpen = true; - popoverRef.present(ev); + + popoverRef.present(new CustomEvent('ionShadowTarget', { + detail: { + ionShadowTarget: ev.target + } + })); await popoverRef.onWillDismiss(); @@ -1362,6 +1366,19 @@ export class Datetime implements ComponentInterface { translucent overlayIndex={1} arrow={false} + onWillPresent={ev => { + /** + * Intersection Observers do not consistently fire between Blink and Webkit + * when toggling the visibility of the popover and trying to scroll the picker + * column to the correct time value. + * + * This will correctly scroll the element position to the correct time value, + * before the popover is fully presented. + */ + const cols = (ev.target! as HTMLElement).querySelectorAll('ion-picker-column-internal'); + // TODO (FW-615): Potentially remove this when intersection observers are fixed in picker column + cols.forEach(col => col.scrollActiveItemIntoView()); + }} style={{ '--offset-y': '-10px' }} diff --git a/core/src/components/datetime/test/position/e2e.ts b/core/src/components/datetime/test/position/e2e.ts new file mode 100644 index 0000000000..0fb39084e0 --- /dev/null +++ b/core/src/components/datetime/test/position/e2e.ts @@ -0,0 +1,27 @@ +import { newE2EPage } from '@stencil/core/testing'; + +describe('datetime: position', () => { + + it('should position the time picker relative to the click target', async () => { + const page = await newE2EPage({ + url: '/src/components/datetime/test/position?ionic:_testing=true' + }); + const screenshotCompares = []; + + const openDateTimeBtn = await page.find('ion-button'); + await openDateTimeBtn.click(); + + screenshotCompares.push(await page.compareScreenshot()); + + const timepickerBtn = await page.find('ion-datetime >>> .time-body'); + await timepickerBtn.click(); + + screenshotCompares.push(await page.compareScreenshot()); + + for (const screenshotCompare of screenshotCompares) { + expect(screenshotCompare).toMatchScreenshot(); + } + + }); + +}); diff --git a/core/src/components/datetime/test/position/index.html b/core/src/components/datetime/test/position/index.html new file mode 100644 index 0000000000..4deba3f335 --- /dev/null +++ b/core/src/components/datetime/test/position/index.html @@ -0,0 +1,51 @@ + + + + + + Datetime - Position + + + + + + + + + + + + Datetime - Position + + + + + Present Popover + + + + + + + + + + + + diff --git a/core/src/components/picker-column-internal/picker-column-internal.tsx b/core/src/components/picker-column-internal/picker-column-internal.tsx index 63a7f1dbe5..fd4c0f7235 100644 --- a/core/src/components/picker-column-internal/picker-column-internal.tsx +++ b/core/src/components/picker-column-internal/picker-column-internal.tsx @@ -1,4 +1,4 @@ -import { Component, ComponentInterface, Element, Event, EventEmitter, Host, Prop, State, Watch, h } from '@stencil/core'; +import { Component, ComponentInterface, Element, Event, EventEmitter, Host, Method, Prop, State, Watch, h } from '@stencil/core'; import { getIonMode } from '../../global/ionic-global'; import { Color } from '../../interface'; @@ -118,7 +118,9 @@ export class PickerColumnInternal implements ComponentInterface { } } - scrollActiveItemIntoView() { + /** @internal */ + @Method() + async scrollActiveItemIntoView() { const activeEl = this.activeItem; if (activeEl) { diff --git a/core/src/components/popover/popover.tsx b/core/src/components/popover/popover.tsx index 508f3683c6..21f5a68d04 100644 --- a/core/src/components/popover/popover.tsx +++ b/core/src/components/popover/popover.tsx @@ -372,7 +372,7 @@ export class Popover implements ComponentInterface, PopoverInterface { * was dispatched. */ @Method() - async present(event?: MouseEvent | TouchEvent | PointerEvent): Promise { + async present(event?: MouseEvent | TouchEvent | PointerEvent | CustomEvent): Promise { if (this.presented) { return; } diff --git a/core/src/components/popover/readme.md b/core/src/components/popover/readme.md index 7639eba5db..90ba567ec3 100644 --- a/core/src/components/popover/readme.md +++ b/core/src/components/popover/readme.md @@ -1022,7 +1022,7 @@ Type: `Promise>` -### `present(event?: MouseEvent | TouchEvent | PointerEvent | undefined) => Promise` +### `present(event?: MouseEvent | TouchEvent | PointerEvent | CustomEvent | undefined) => Promise` Present the popover overlay after it has been created. Developers can pass a mouse, touch, or pointer event