Skip to content

Commit cfeed00

Browse files
authored
fix(ui5-popover): set fallback placement when no place to popup (#1467)
**Issue** When the opener element is taking full width and the placement is set to "Right" (same for "Left"), the popover tried to open on the opposite side, but both sides are not possible and the popover remains cut off with only its arrow visible. **Solution** Now, it checks "Bottom" and "Top" sides as well. **Note1:** The fallback is applied before the allow-target-overlap to be taken into account, which means that Popover with placement "Right" or "Left", will try to open on all sides, before overlapping its target. **Note2:** The fallback is used currently for "Left" and "Right' only for two reasons: (1) it fixes the issue that was originally reported by the MDK colleagues and (2) all the components, that use popover internally (f.e DatePicker) have placement="Bottom" and the change would have bigger impact, that I would avoid before release. **Note3:** The full solution is to have this fallback for all sides + placement type "Auto" that should be the default value and would try to open on "Right" first. Fixes: #1395
1 parent d864dec commit cfeed00

File tree

2 files changed

+156
-7
lines changed

2 files changed

+156
-7
lines changed

packages/main/src/Popover.js

+26-6
Original file line numberDiff line numberDiff line change
@@ -628,6 +628,28 @@ class Popover extends UI5Element {
628628
};
629629
}
630630

631+
/**
632+
* Fallbacks to new placement, prioritizing <code>Left</code> and <code>Right</code> placements.
633+
* @private
634+
*/
635+
fallbackPlacement(clientWidth, clientHeight, targetRect, popoverSize) {
636+
if (targetRect.left > popoverSize.width) {
637+
return PopoverPlacementType.Left;
638+
}
639+
640+
if (clientWidth - targetRect.right > targetRect.left) {
641+
return PopoverPlacementType.Right;
642+
}
643+
644+
if (clientHeight - targetRect.bottom > popoverSize.height) {
645+
return PopoverPlacementType.Bottom;
646+
}
647+
648+
if (clientHeight - targetRect.bottom < targetRect.top) {
649+
return PopoverPlacementType.Top;
650+
}
651+
}
652+
631653
getActualPlacementType(targetRect, popoverSize) {
632654
const placementType = this.placementType;
633655
let actualPlacementType = placementType;
@@ -649,15 +671,13 @@ class Popover extends UI5Element {
649671
}
650672
break;
651673
case PopoverPlacementType.Left:
652-
if (targetRect.left < popoverSize.width
653-
&& targetRect.left < clientWidth - targetRect.right) {
654-
actualPlacementType = PopoverPlacementType.Right;
674+
if (targetRect.left < popoverSize.width) {
675+
actualPlacementType = this.fallbackPlacement(clientWidth, clientHeight, targetRect, popoverSize) || placementType;
655676
}
656677
break;
657678
case PopoverPlacementType.Right:
658-
if (clientWidth - targetRect.right < popoverSize.width
659-
&& clientWidth - targetRect.right < targetRect.left) {
660-
actualPlacementType = PopoverPlacementType.Left;
679+
if (clientWidth - targetRect.right < popoverSize.width) {
680+
actualPlacementType = this.fallbackPlacement(clientWidth, clientHeight, targetRect, popoverSize) || placementType;
661681
}
662682
break;
663683
}

packages/main/test/pages/Popover.html

+130-1
Original file line numberDiff line numberDiff line change
@@ -185,8 +185,114 @@
185185
</div>
186186
</ui5-popover>
187187

188-
<script>
188+
<br>
189+
<br>
190+
<ui5-title>placement-type="Right", but no space on the right</ui5-title>
191+
<p>No space on the Right, try Left, Bottom, Top</p>
192+
<ui5-button id="btnFullWidth" style="width: 100%">Click me !</ui5-button>
193+
<br>
194+
<br>
195+
<ui5-title>placement-type="Right" (default) and allow-target-overlap=true </ui5-title>
196+
<p>No space on the right, try Left, Bottom, Top and if nothing works, the target will be overlapped</p>
197+
<ui5-button id="btnFullWidthTargetOverlap" style="width: 100%">Click me !</ui5-button>
189198

199+
<br>
200+
<br>
201+
<ui5-title>placement-type="Left", but no space on the left</ui5-title>
202+
<p>No space on the Left, try Right, Bottom, Top</p>
203+
<ui5-button id="btnFullWidthLeft" style="width: 100%">Click me !</ui5-button>
204+
205+
<br>
206+
<br>
207+
<ui5-title>placement-type="Left" and allow-target-overlap=true</ui5-title>
208+
<p>No space on the right, try Roght, Bottom, Top and if nothing works, the target will be overlapped</p>
209+
<ui5-button id="btnFullWidthLeftTargetOverlap" style="width: 100%">Click me !</ui5-button>
210+
211+
<br>
212+
<br>
213+
<ui5-title>placement-type="Top"</ui5-title>
214+
<ui5-button id="btnFullWidthTop" style="width: 100%">Click me !</ui5-button>
215+
216+
<br>
217+
<br>
218+
<ui5-title>placement-type="Bottom"</ui5-title>
219+
<ui5-button id="btnFullWidthBottom" style="width: 100%">Click me !</ui5-button>
220+
221+
<ui5-popover header-text="My Heading" id="popFullWidth" style="width: 300px" >
222+
<div slot="header">
223+
<ui5-button id="first-focusable">I am in the header</ui5-button>
224+
</div>
225+
226+
<ui5-list>
227+
<ui5-li>Hello</ui5-li>
228+
<ui5-li>World</ui5-li>
229+
<ui5-li>Again</ui5-li>
230+
</ui5-list>
231+
</ui5-popover>
232+
233+
<ui5-popover allow-target-overlap header-text="My Heading" id="popFullWidthTargetOverlap" style="width: 300px" >
234+
<div slot="header">
235+
<ui5-button id="first-focusable">I am in the header</ui5-button>
236+
</div>
237+
238+
<ui5-list>
239+
<ui5-li>Hello</ui5-li>
240+
<ui5-li>World</ui5-li>
241+
<ui5-li>Again</ui5-li>
242+
</ui5-list>
243+
</ui5-popover>
244+
245+
246+
<ui5-popover header-text="My Heading" id="popFullWidthLeft" style="width: 300px" placement-type="Left">
247+
<div slot="header">
248+
<ui5-button id="first-focusable">I am in the header</ui5-button>
249+
</div>
250+
251+
<ui5-list>
252+
<ui5-li>Hello</ui5-li>
253+
<ui5-li>World</ui5-li>
254+
<ui5-li>Again</ui5-li>
255+
</ui5-list>
256+
</ui5-popover>
257+
258+
<ui5-popover header-text="My Heading" id="popFullWidthLeftTargetOverlap" style="width: 300px" placement-type="Left" allow-target-overlap>
259+
<div slot="header">
260+
<ui5-button id="first-focusable">I am in the header</ui5-button>
261+
</div>
262+
263+
<ui5-list>
264+
<ui5-li>Hello</ui5-li>
265+
<ui5-li>World</ui5-li>
266+
<ui5-li>Again</ui5-li>
267+
</ui5-list>
268+
</ui5-popover>
269+
270+
271+
<ui5-popover header-text="My Heading" id="popFullWidthTop" style="width: 300px" placement-type="Top">
272+
<div slot="header">
273+
<ui5-button id="first-focusable">I am in the header</ui5-button>
274+
</div>
275+
276+
<ui5-list>
277+
<ui5-li>Hello</ui5-li>
278+
<ui5-li>World</ui5-li>
279+
<ui5-li>Again</ui5-li>
280+
</ui5-list>
281+
</ui5-popover>
282+
283+
<ui5-popover header-text="My Heading" id="popFullWidthBottom" style="width: 300px" placement-type="Bottom">
284+
<div slot="header">
285+
<ui5-button id="first-focusable">I am in the header</ui5-button>
286+
</div>
287+
288+
<ui5-list>
289+
<ui5-li>Hello</ui5-li>
290+
<ui5-li>World</ui5-li>
291+
<ui5-li>Again</ui5-li>
292+
</ui5-list>
293+
</ui5-popover>
294+
295+
<script>
190296
btn.addEventListener('click', function (event) {
191297
pop.openBy(btn);
192298
});
@@ -223,6 +329,29 @@
223329
document.getElementById("big-popover").openBy(event.target);
224330
});
225331

332+
btnFullWidth.addEventListener('click', function (event) {
333+
popFullWidth.openBy(btnFullWidth);
334+
});
335+
336+
btnFullWidthTargetOverlap.addEventListener('click', function (event) {
337+
popFullWidthTargetOverlap.openBy(btnFullWidthTargetOverlap);
338+
});
339+
340+
btnFullWidthLeft.addEventListener('click', function (event) {
341+
popFullWidthLeft.openBy(btnFullWidthLeft);
342+
});
343+
344+
btnFullWidthLeftTargetOverlap.addEventListener('click', function (event) {
345+
popFullWidthLeftTargetOverlap.openBy(btnFullWidthLeftTargetOverlap);
346+
});
347+
348+
btnFullWidthTop.addEventListener('click', function (event) {
349+
popFullWidthTop.openBy(btnFullWidthTop);
350+
});
351+
352+
btnFullWidthBottom.addEventListener('click', function (event) {
353+
popFullWidthBottom.openBy(btnFullWidthBottom);
354+
});
226355
</script>
227356
</body>
228357

0 commit comments

Comments
 (0)