Skip to content

Commit 9097a1d

Browse files
ShaneKShaneK
committed
Trying to improve accuracy
1 parent edc202d commit 9097a1d

3 files changed

Lines changed: 90 additions & 66 deletions

File tree

‎core/src/components/popover/animations/ios.enter.ts‎

Lines changed: 66 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,37 @@ export const iosEnterAnimation = (baseEl: HTMLElement, opts?: any): Animation =>
9393
arrowHeight
9494
);
9595

96+
/**
97+
* Safe area CSS variable adjustments.
98+
* When the popover is positioned near an edge, we add the corresponding
99+
* safe-area inset to ensure the popover doesn't overlap with system UI
100+
* (status bars, home indicators, navigation bars on Android API 36+, etc.)
101+
*/
102+
const safeAreaTop = ' + var(--ion-safe-area-top, 0px)';
103+
const safeAreaBottom = ' + var(--ion-safe-area-bottom, 0px)';
104+
const safeAreaLeft = ' + var(--ion-safe-area-left, 0px)';
105+
const safeAreaRight = ' - var(--ion-safe-area-right, 0px)';
106+
107+
let topValue = `${top}px`;
108+
let bottomValue = bottom !== undefined ? `${bottom}px` : undefined;
109+
let leftValue = `${left}px`;
110+
111+
if (checkSafeAreaTop) {
112+
topValue = `${top}px${safeAreaTop}`;
113+
}
114+
if (checkSafeAreaBottom && bottomValue !== undefined) {
115+
bottomValue = `${bottom}px${safeAreaBottom}`;
116+
}
117+
if (checkSafeAreaLeft) {
118+
leftValue = `${left}px${safeAreaLeft}`;
119+
}
120+
if (checkSafeAreaRight) {
121+
leftValue = `${left}px${safeAreaRight}`;
122+
}
123+
96124
const baseAnimation = createAnimation();
97125
const backdropAnimation = createAnimation();
126+
const arrowAnimation = createAnimation();
98127
const contentAnimation = createAnimation();
99128

100129
backdropAnimation
@@ -109,71 +138,54 @@ export const iosEnterAnimation = (baseEl: HTMLElement, opts?: any): Animation =>
109138
// The Chromium team stated that this behavior is expected and not a bug. The element animating opacity creates a backdrop root for the backdrop-filter.
110139
// To get around this, instead of animating the wrapper, animate both the arrow and content.
111140
// https://bugs.chromium.org/p/chromium/issues/detail?id=1148826
112-
contentAnimation
113-
.addElement(root.querySelector('.popover-arrow')!)
114-
.addElement(root.querySelector('.popover-content')!)
115-
.fromTo('opacity', 0.01, 1);
116141
// TODO(FW-4376) Ensure that arrow also blurs when translucent
142+
if (arrowEl !== null) {
143+
arrowAnimation.addElement(arrowEl).fromTo('opacity', 0.01, 1);
144+
}
117145

118-
return baseAnimation
119-
.easing('ease')
120-
.duration(100)
146+
contentAnimation
147+
.addElement(contentEl)
121148
.beforeAddWrite(() => {
122-
if (size === 'cover') {
123-
baseEl.style.setProperty('--width', `${contentWidth}px`);
124-
}
125-
126-
if (addPopoverBottomClass) {
127-
baseEl.classList.add('popover-bottom');
128-
}
129-
130-
/**
131-
* Safe area CSS variable adjustments.
132-
* When the popover is positioned near an edge, we add the corresponding
133-
* safe-area inset to ensure the popover doesn't overlap with system UI
134-
* (status bars, home indicators, navigation bars on Android API 36+, etc.)
135-
*/
136-
const safeAreaTop = ' + var(--ion-safe-area-top, 0)';
137-
const safeAreaBottom = ' + var(--ion-safe-area-bottom, 0)';
138-
const safeAreaLeft = ' + var(--ion-safe-area-left, 0)';
139-
const safeAreaRight = ' - var(--ion-safe-area-right, 0)';
140-
141-
let topValue = `${top}px`;
142-
let bottomValue = bottom !== undefined ? `${bottom}px` : undefined;
143-
let leftValue = `${left}px`;
144-
145-
if (checkSafeAreaTop) {
146-
topValue = `${top}px${safeAreaTop}`;
147-
}
148-
if (checkSafeAreaBottom && bottomValue !== undefined) {
149-
bottomValue = `${bottom}px${safeAreaBottom}`;
150-
}
151-
if (checkSafeAreaLeft) {
152-
leftValue = `${left}px${safeAreaLeft}`;
153-
}
154-
if (checkSafeAreaRight) {
155-
leftValue = `${left}px${safeAreaRight}`;
156-
}
149+
contentEl.style.setProperty('top', `calc(${topValue} + var(--offset-y, 0px))`);
150+
contentEl.style.setProperty('left', `calc(${leftValue} + var(--offset-x, 0px))`);
151+
contentEl.style.setProperty('transform-origin', `${originY} ${originX}`);
157152

158153
if (bottomValue !== undefined) {
159154
contentEl.style.setProperty('bottom', `calc(${bottomValue})`);
160155
/**
161156
* When both top and bottom are explicitly constrained (isFullyConstrained),
162-
* we need to override the height: var(--height) style to allow the
163-
* top/bottom constraint to determine the height.
157+
* we need to explicitly calculate the height to ensure the popover
158+
* fits within the safe area boundaries.
164159
*
165-
* We only do this when fully constrained because setting height: unset
166-
* when only bottom is set (without explicit top) would result in an
167-
* incorrectly sized popover.
160+
* Using CSS calc with 100vh minus top and bottom values ensures the
161+
* popover height respects both safe areas. We also override max-height
162+
* to prevent it from interfering with the calculated height.
168163
*/
169164
if (isFullyConstrained) {
170-
contentEl.style.setProperty('height', 'unset');
165+
/**
166+
* Wrap topValue and bottomValue in parentheses to ensure correct
167+
* order of operations in the CSS calc. Without parentheses, the
168+
* safe-area additions would have wrong signs.
169+
*/
170+
const heightCalc = `calc(100vh - (${topValue}) - (${bottomValue}) - var(--offset-y, 0px))`;
171+
contentEl.style.setProperty('height', heightCalc);
172+
contentEl.style.setProperty('max-height', heightCalc);
171173
}
172174
}
175+
})
176+
.fromTo('opacity', 0.01, 1);
173177

174-
contentEl.style.setProperty('top', `calc(${topValue} + var(--offset-y, 0))`);
175-
contentEl.style.setProperty('left', `calc(${leftValue} + var(--offset-x, 0))`);
176-
contentEl.style.setProperty('transform-origin', `${originY} ${originX}`);
178+
return baseAnimation
179+
.easing('ease')
180+
.duration(100)
181+
.beforeAddWrite(() => {
182+
if (size === 'cover') {
183+
baseEl.style.setProperty('--width', `${contentWidth}px`);
184+
}
185+
186+
if (addPopoverBottomClass) {
187+
baseEl.classList.add('popover-bottom');
188+
}
177189

178190
if (arrowEl !== null) {
179191
const didAdjustBounds = results.top !== top || results.left !== left;
@@ -184,12 +196,12 @@ export const iosEnterAnimation = (baseEl: HTMLElement, opts?: any): Animation =>
184196
const showArrow = shouldShowArrow(side, didAdjustBounds, ev, trigger) && !isFullyConstrained;
185197

186198
if (showArrow) {
187-
arrowEl.style.setProperty('top', `calc(${arrowTop}px + var(--offset-y, 0))`);
188-
arrowEl.style.setProperty('left', `calc(${arrowLeft}px + var(--offset-x, 0))`);
199+
arrowEl.style.setProperty('top', `calc(${arrowTop}px + var(--offset-y, 0px))`);
200+
arrowEl.style.setProperty('left', `calc(${arrowLeft}px + var(--offset-x, 0px))`);
189201
} else {
190202
arrowEl.style.setProperty('display', 'none');
191203
}
192204
}
193205
})
194-
.addAnimation([backdropAnimation, contentAnimation]);
206+
.addAnimation([backdropAnimation, arrowAnimation, contentAnimation]);
195207
};

‎core/src/components/popover/animations/md.enter.ts‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -137,15 +137,22 @@ export const mdEnterAnimation = (baseEl: HTMLElement, opts?: any): Animation =>
137137
contentEl.style.setProperty('bottom', `calc(${bottomValue})`);
138138
/**
139139
* When both top and bottom are explicitly constrained (isFullyConstrained),
140-
* we need to override the height: var(--height) style to allow the
141-
* top/bottom constraint to determine the height.
140+
* we need to explicitly calculate the height to ensure the popover
141+
* fits within the safe area boundaries.
142142
*
143-
* We only do this when fully constrained because setting height: unset
144-
* when only bottom is set (without explicit top) would result in an
145-
* incorrectly sized popover.
143+
* Using CSS calc with 100vh minus top and bottom values ensures the
144+
* popover height respects both safe areas. We also override max-height
145+
* to prevent it from interfering with the calculated height.
146146
*/
147147
if (isFullyConstrained) {
148-
contentEl.style.setProperty('height', 'unset');
148+
/**
149+
* Wrap topValue and bottomValue in parentheses to ensure correct
150+
* order of operations in the CSS calc. Without parentheses, the
151+
* safe-area additions would have wrong signs.
152+
*/
153+
const heightCalc = `calc(100vh - (${topValue}) - (${bottomValue}) - var(--offset-y, 0px))`;
154+
contentEl.style.setProperty('height', heightCalc);
155+
contentEl.style.setProperty('max-height', heightCalc);
149156
}
150157
}
151158
})

‎core/src/components/popover/utils.ts‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -927,13 +927,13 @@ export const calculateWindowAdjustment = (
927927
*
928928
* When checkSafeAreaTop is true, the CSS will add safe-area-top to the
929929
* top position, pushing the popover down. Since we don't know the exact
930-
* CSS safe-area value, we use a threshold that accounts for likely
931-
* safe-area sizes. This only triggers when:
932-
* 1. We're already applying safe-area-top (checkSafeAreaTop), and
933-
* 2. The popover is close enough to overflowing that any safe-area
934-
* would push it past the viewport
930+
* CSS safe-area value at runtime, we use a conservative threshold that
931+
* accounts for typical safe-area sizes (usually 40-50px). By checking
932+
* against (safeAreaMargin * 2), we ensure that:
933+
* 1. Any popover close to the viewport boundary gets constrained
934+
* 2. The safe-area CSS variables have room to be applied without overflow
935935
*/
936-
if (checkSafeAreaTop && top + contentHeight > bodyHeight - safeAreaMargin - bodyPadding) {
936+
if (checkSafeAreaTop && top + contentHeight > bodyHeight - safeAreaMargin * 2 - bodyPadding) {
937937
bottom = bodyPadding;
938938
checkSafeAreaBottom = true;
939939
isFullyConstrained = true;
@@ -965,8 +965,13 @@ export const calculateWindowAdjustment = (
965965
/**
966966
* Set a bottom constraint to push the popover up out of the safe-area zone.
967967
* The animation will add the safe-area CSS variable to this value.
968+
*
969+
* We also set isFullyConstrained so that height: unset is applied,
970+
* allowing the bottom constraint to actually take effect (otherwise
971+
* the explicit height would override the bottom constraint).
968972
*/
969973
bottom = bodyPadding;
974+
isFullyConstrained = true;
970975
}
971976
if (top < safeAreaMargin) {
972977
checkSafeAreaTop = true;

0 commit comments

Comments
 (0)