Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/popover/animations/ios.enter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ export const iosEnterAnimation = (baseEl: HTMLElement, opts: any = {}): Animatio
results.referenceCoordinates,
referenceSizeEl?.getBoundingClientRect(),
isReplace,
opts.preserveHorizontalAlignment === true,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 Vertical Bars のオプトイン指定箇所を確認

説明にあるテーマ側の preserveHorizontalAlignment: true 指定は、この変更には含まれていません。投影コントロールの配置を維持するため、対応するテーマ側の変更が適用されるか確認してください。

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

);
// A replacing surface grows inward from the button's edge, not its center.
const preferredLeft =
Expand Down
27 changes: 24 additions & 3 deletions src/popover/utils.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,31 @@ import { calculateWindowAdjustment, getIndexOfItem, getNextItem, getPopoverPosit

describe('popover utilities', () => {
it.each(['left', 'right', 'start', 'end'] as const)('preserves the vertical anchor alignment for side=%s', (side) => {
const position = calculateWindowAdjustment(side, 120, 100, 5, 440, 636, 200, 52, 8, 'right', 'center');
const position = calculateWindowAdjustment(
side,
120,
100,
5,
440,
636,
200,
52,
8,
'right',
'center',
undefined,
undefined,
false,
true,
);
expect(position.top).toBe(120);
expect(calculateWindowAdjustment(side, -4, 100, 5, 440, 636, 200, 52, 8, 'right', 'center').top).toBe(5);
expect(calculateWindowAdjustment(side, 620, 100, 5, 440, 636, 200, 52, 8, 'right', 'center').top).toBe(579);
expect(calculateWindowAdjustment(side, 120, 100, 5, 440, 636, 200, 52, 8, 'right', 'center').top).toBe(128);
expect(
calculateWindowAdjustment(side, -4, 100, 5, 440, 636, 200, 52, 8, 'right', 'center', undefined, undefined, false, true).top,
).toBe(5);
expect(
calculateWindowAdjustment(side, 620, 100, 5, 440, 636, 200, 52, 8, 'right', 'center', undefined, undefined, false, true).top,
).toBe(579);
});

it('navigates only relative to ion-item elements', () => {
Expand Down
4 changes: 3 additions & 1 deletion src/popover/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -612,11 +612,13 @@ export const calculateWindowAdjustment = (
triggerCoordinates?: ReferenceCoordinates,
eventElementRect?: DOMRect,
isReplace: boolean = false,
preserveHorizontalAlignment: boolean = false,
): PopoverStyles => {
const triggerTop = triggerCoordinates ? triggerCoordinates.top + triggerCoordinates.height : bodyHeight / 2 - contentHeight / 2;
const triggerHeight = triggerCoordinates ? triggerCoordinates.height : 0;
let left = coordLeft;
const horizontal = side === 'left' || side === 'right' || side === 'start' || side === 'end';
// Projected rail controls opt in; ordinary popovers retain their existing margin.
const horizontal = preserveHorizontalAlignment && (side === 'left' || side === 'right' || side === 'start' || side === 'end');
let top = !isReplace ? coordTop + (horizontal ? 0 : POPOVER_IOS_BODY_MARGIN) : coordTop - triggerHeight;
if (horizontal && !isReplace) top = Math.max(bodyPadding, Math.min(top, bodyHeight - bodyPadding - contentHeight));
Comment on lines +621 to 623

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 通常の横配置ポップオーバーが画面外に切れる

横配置でオプションを指定しないと、8px の余白だけが加わり、従来の上下方向のクランプが働きません。画面端付近ではポップオーバーの内容が画面外に出ます。

Learn more

この関数はポップオーバーの座標を画面内に調整します。横配置では従来、bodyPadding を使って上下端を制限していました。今回、余白の有無とクランプの有無を同じ horizontal 条件で切り替えたため、通常の横配置でクランプまで失われます。トリガーが下端付近にあると、コンテンツの下部が見えなくなります。

Example: 画面高 636px、コンテンツ高 52px、coordTop が 620px、side が right、bodyPadding が 5px の場合、デフォルトの top は 628px になります。従来は 579px に制限され、コンテンツ全体が表示されました。

Recommended fix: 横配置かどうかの判定と、余白を省くためのオプトイン判定を分離してください。通常配置には 8px の余白を残しつつ、余白を適用した後の座標も上下の画面内に制限し、オプトイン時の配置も維持してください。

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

let bottom;
Expand Down
Loading