Skip to content

Commit

Permalink
fix(datetime): timepicker popover will position relative to click target
Browse files Browse the repository at this point in the history
Resolves #24531, #24415, #24585

Co-authored-by: mixalbl4 <[email protected]>
  • Loading branch information
sean-perkins and mixalbl4-127 committed Jan 24, 2022
1 parent 43c5977 commit 82a7212
Show file tree
Hide file tree
Showing 8 changed files with 105 additions and 8 deletions.
2 changes: 1 addition & 1 deletion core/api.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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<boolean>
ion-popover,method,onDidDismiss,onDidDismiss<T = any>() => Promise<OverlayEventDetail<T>>
ion-popover,method,onWillDismiss,onWillDismiss<T = any>() => Promise<OverlayEventDetail<T>>
ion-popover,method,present,present(event?: MouseEvent | TouchEvent | PointerEvent | undefined) => Promise<void>
ion-popover,method,present,present(event?: MouseEvent | TouchEvent | PointerEvent | CustomEvent<any> | undefined) => Promise<void>
ion-popover,event,didDismiss,OverlayEventDetail<any>,true
ion-popover,event,didPresent,void,true
ion-popover,event,ionPopoverDidDismiss,OverlayEventDetail<any>,true
Expand Down
3 changes: 2 additions & 1 deletion core/src/components.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<void>;
/**
* The selected option in the picker.
*/
Expand Down Expand Up @@ -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<void>;
"present": (event?: MouseEvent | TouchEvent | PointerEvent | CustomEvent<any> | undefined) => Promise<void>;
/**
* 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.
*/
Expand Down
20 changes: 18 additions & 2 deletions core/src/components/datetime/datetime.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -1362,6 +1366,18 @@ 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');
cols.forEach(col => col.scrollActiveItemIntoView());
}}
style={{
'--offset-y': '-10px'
}}
Expand Down
27 changes: 27 additions & 0 deletions core/src/components/datetime/test/position/e2e.ts
Original file line number Diff line number Diff line change
@@ -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();
}

});

});
51 changes: 51 additions & 0 deletions core/src/components/datetime/test/position/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
<!DOCTYPE html>
<html lang="en" dir="ltr">

<head>
<meta charset="UTF-8">
<title>Datetime - Position</title>
<meta name="viewport" content="width=device-width, initial-scale=1.0, minimum-scale=1.0">
<link href="../../../../../css/ionic.bundle.css" rel="stylesheet">
<link href="../../../../../scripts/testing/styles.css" rel="stylesheet">
<script src="../../../../../scripts/testing/scripts.js"></script>
<script type="module" src="../../../../../dist/ionic/ionic.esm.js"></script>
</head>

<body>
<ion-app>
<ion-header translucent="true">
<ion-toolbar>
<ion-title>Datetime - Position</ion-title>
</ion-toolbar>
</ion-header>
<ion-content class="ion-padding">

<ion-button onclick="presentPopover(defaultPopover, event)">Present Popover</ion-button>
<ion-popover class="datetime-popover" id="default-popover">
<ion-datetime></ion-datetime>
</ion-popover>

</ion-content>

<script>
const defaultPopover = document.querySelector('ion-popover#default-popover');
const presentPopover = (popover, ev) => {
popover.event = ev;
popover.showBackdrop = false;
popover.isOpen = true;

const dismiss = () => {
popover.isOpen = false;
popover.event = undefined;

popover.removeEventListener('didDismiss', dismiss);
}

popover.addEventListener('didDismiss', dismiss);
}
</script>

</ion-app>
</body>

</html>
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -118,7 +118,9 @@ export class PickerColumnInternal implements ComponentInterface {
}
}

scrollActiveItemIntoView() {
/** @internal */
@Method()
async scrollActiveItemIntoView() {
const activeEl = this.activeItem;

if (activeEl) {
Expand Down
2 changes: 1 addition & 1 deletion core/src/components/popover/popover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ export class Popover implements ComponentInterface, PopoverInterface {
* was dispatched.
*/
@Method()
async present(event?: MouseEvent | TouchEvent | PointerEvent): Promise<void> {
async present(event?: MouseEvent | TouchEvent | PointerEvent | CustomEvent): Promise<void> {
if (this.presented) {
return;
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/components/popover/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -1022,7 +1022,7 @@ Type: `Promise<OverlayEventDetail<T>>`



### `present(event?: MouseEvent | TouchEvent | PointerEvent | undefined) => Promise<void>`
### `present(event?: MouseEvent | TouchEvent | PointerEvent | CustomEvent<any> | undefined) => Promise<void>`

Present the popover overlay after it has been created.
Developers can pass a mouse, touch, or pointer event
Expand Down

0 comments on commit 82a7212

Please sign in to comment.