Skip to content

Commit 8135904

Browse files
TeodorTaushanovTeodorTaushanov
authored
fix(ui5-popover): fix horizontal placement (Start and End) in rtl mode (#12606)
fix horizontal placement (Start and End) in rtl mode fix usage of placement and horizontalAlign properties in the framework add new PopoverActualHorizontalAlign and PopoverActualPlacement enums
1 parent 142d026 commit 8135904

18 files changed

+246
-128
lines changed

‎packages/fiori/src/ShellBar.ts‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ import type {
4141
UI5CustomEvent,
4242
} from "@ui5/webcomponents-base";
4343
import type ListItemBase from "@ui5/webcomponents/dist/ListItemBase.js";
44-
import type PopoverHorizontalAlign from "@ui5/webcomponents/dist/types/PopoverHorizontalAlign.js";
4544
import throttle from "@ui5/webcomponents-base/dist/util/throttle.js";
4645
import { getScopedVarName } from "@ui5/webcomponents-base/dist/CustomElementsScope.js";
4746
import getActiveElement from "@ui5/webcomponents-base/dist/util/getActiveElement.js";
@@ -1524,10 +1523,6 @@ class ShellBar extends UI5Element {
15241523
return this.primaryTitle || this.showLogoInMenuButton;
15251524
}
15261525

1527-
get popoverHorizontalAlign(): `${PopoverHorizontalAlign}` {
1528-
return this.effectiveDir === "rtl" ? "Start" : "End";
1529-
}
1530-
15311526
get hasAssistant() {
15321527
return !!this.assistant.length;
15331528
}

‎packages/fiori/src/ShellBarPopoverTemplate.tsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import Popover from "@ui5/webcomponents/dist/Popover.js";
22
import List from "@ui5/webcomponents/dist/List.js";
3+
import PopoverHorizontalAlign from "@ui5/webcomponents/dist/types/PopoverHorizontalAlign.js";
34
import ListItemStandard from "@ui5/webcomponents/dist/ListItemStandard.js";
45
import type ShellBar from "./ShellBar.js";
56

@@ -21,7 +22,7 @@ export default function PopoversTemplate(this: ShellBar) {
2122
<Popover class="ui5-shellbar-overflow-popover"
2223
placement="Bottom"
2324
preventInitialFocus={true}
24-
horizontalAlign={this.popoverHorizontalAlign}
25+
horizontalAlign={PopoverHorizontalAlign.End}
2526
hideArrow={true}
2627
onBeforeOpen={this._overflowPopoverBeforeOpen}
2728
onClose={this._overflowPopoverAfterClose}

‎packages/main/cypress/specs/Popover.cy.tsx‎

Lines changed: 101 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -266,40 +266,40 @@ describe("Accessibility", () => {
266266
<div slot="header" />
267267
</Popover>
268268
);
269-
269+
270270
cy.get("[ui5-popover]").invoke("removeAttr", "accessible-name");
271-
271+
272272
cy.get("[ui5-popover]")
273273
.shadow()
274274
.find(".ui5-popup-root")
275275
.should("have.attr", "aria-labelledby");
276-
276+
277277
cy.get("[ui5-popover]")
278278
.shadow()
279279
.find(".ui5-popup-root")
280280
.should("not.have.attr", "aria-label");
281281
});
282-
282+
283283
it("should use aria-label when accessible-name attribute is set dynamically", () => {
284284
cy.mount(
285285
<Popover accessibleName="This popover is important">
286286
<div slot="header" />
287287
</Popover>
288288
);
289-
289+
290290
cy.get("[ui5-popover]").invoke("attr", "accessible-name", "text");
291-
291+
292292
cy.get("[ui5-popover]")
293293
.shadow()
294294
.find(".ui5-popup-root")
295295
.should("not.have.attr", "aria-labelledby");
296-
296+
297297
cy.get("[ui5-popover]")
298298
.shadow()
299299
.find(".ui5-popup-root")
300300
.should("have.attr", "aria-label");
301301
});
302-
302+
303303

304304
it("tests accessible-name-ref", () => {
305305
cy.mount(
@@ -652,25 +652,25 @@ describe("Popover opener", () => {
652652
</Popover>
653653
</>
654654
);
655-
655+
656656
cy.get("#first-focusable").should("be.focused");
657-
657+
658658
cy.realPress("Tab");
659659
cy.wait(500);
660660
cy.get("#li1").should("be.focused");
661661
cy.get("#first-focusable").should("not.be.focused");
662-
662+
663663
cy.realPress("Tab");
664-
664+
665665
cy.get("#first-focusable").should("be.focused");
666-
666+
667667
cy.realPress("Tab");
668668
cy.realPress("Tab");
669-
669+
670670
cy.get("#first-focusable").should("be.focused");
671-
671+
672672
cy.realPress("Escape");
673-
673+
674674
cy.get("[ui5-popover]").should("not.be.visible");
675675
});
676676

@@ -967,7 +967,7 @@ describe("Popover opener", () => {
967967

968968
pop.addEventListener("ui5-before-open", async () => {
969969
const applyFocusResult = pop.applyFocus();
970-
pop.remove();
970+
pop.remove();
971971

972972
try {
973973
await applyFocusResult;
@@ -1020,31 +1020,31 @@ describe("Popover opener", () => {
10201020
const container = document.createElement("div");
10211021
container.id = "container";
10221022
root[0].appendChild(container);
1023-
1023+
10241024
const shadowRoot = container.attachShadow({ mode: "open" });
1025-
1025+
10261026
const opener = document.createElement("ui5-button");
10271027
opener.setAttribute("id", "lnk");
10281028
opener.textContent = "Open Popover";
10291029
shadowRoot.appendChild(opener);
1030-
1030+
10311031
const popover = document.createElement("ui5-popover");
10321032
popover.setAttribute("id", "pop");
10331033
popover.setAttribute("header-text", "Popover in Shadow Root");
10341034
popover.setAttribute("opener", "lnk");
1035-
1035+
10361036
const content = document.createElement("div");
10371037
content.textContent = "Popover content";
10381038
popover.appendChild(content);
1039-
1039+
10401040
shadowRoot.appendChild(popover);
10411041
});
1042-
1042+
10431043
cy.get("#container")
10441044
.shadow()
10451045
.find("#lnk")
10461046
.realClick();
1047-
1047+
10481048
cy.get("#container")
10491049
.shadow()
10501050
.find("#pop")
@@ -1053,7 +1053,7 @@ describe("Popover opener", () => {
10531053
cy.get("#container").then(container => {
10541054
container.remove();
10551055
});
1056-
});
1056+
});
10571057

10581058
it("tests opener set as ID in window.document, while popover is in a shadow root", () => {
10591059
cy.mount(
@@ -1421,6 +1421,82 @@ describe("Placement", () => {
14211421
expect(top).to.be.lt(100)
14221422
});
14231423
});
1424+
1425+
it("placement=Start in RTL", () => {
1426+
cy.mount(
1427+
<div dir="rtl">
1428+
<Button id="btnStart" style="position: absolute; left: 50%; top: 50%; transform: translate(-50%, -50%);">Open</Button>
1429+
<Popover id="popoverStart"
1430+
headerText="Start Placement"
1431+
opener="btnStart"
1432+
placement="Start">
1433+
<div style="width: 150px; padding: 10px;">
1434+
Popover with Start placement in RTL mode
1435+
</div>
1436+
</Popover>
1437+
</div>
1438+
);
1439+
1440+
cy.get("[ui5-popover]").invoke("prop", "open", true);
1441+
1442+
cy.get<Popover>("[ui5-popover]").should("be.visible");
1443+
1444+
// wait for the popover to be positioned
1445+
// eslint-disable-next-line cypress/no-unnecessary-waiting
1446+
cy.wait(200);
1447+
1448+
let popover;
1449+
cy.get('[ui5-popover]')
1450+
.then($popover => {
1451+
popover = $popover;
1452+
});
1453+
1454+
cy.get('#btnStart').should($opener => {
1455+
const popoverRect = popover[0].getBoundingClientRect();
1456+
const openerRect = $opener[0].getBoundingClientRect();
1457+
1458+
// In RTL mode, Start placement should position popover to the right of the opener
1459+
expect(popoverRect.left).to.be.greaterThan(openerRect.right - 5);
1460+
});
1461+
});
1462+
1463+
it("placement=End in RTL", () => {
1464+
cy.mount(
1465+
<div dir="rtl">
1466+
<Button id="btnEnd" style="position: absolute; left: 50%; top: 50%; transform: translate(-50%, -50%);">Open</Button>
1467+
<Popover id="popoverEndg"
1468+
headerText="End Placement"
1469+
opener="btnEnd"
1470+
placement="End">
1471+
<div style="width: 150px; padding: 10px;">
1472+
Popover with End placement in RTL mode
1473+
</div>
1474+
</Popover>
1475+
</div>
1476+
);
1477+
1478+
cy.get("[ui5-popover]").invoke("prop", "open", true);
1479+
1480+
cy.get<Popover>("[ui5-popover]").should("be.visible");
1481+
1482+
// wait for the popover to be positioned
1483+
// eslint-disable-next-line cypress/no-unnecessary-waiting
1484+
cy.wait(200);
1485+
1486+
let popover;
1487+
cy.get('[ui5-popover]')
1488+
.then($popover => {
1489+
popover = $popover;
1490+
});
1491+
1492+
cy.get('#btnEnd').should($opener => {
1493+
const popoverRect = popover[0].getBoundingClientRect();
1494+
const openerRect = $opener[0].getBoundingClientRect();
1495+
1496+
// In RTL mode, End placement should position popover to the left of the opener
1497+
expect(popoverRect.right).to.be.lessThan(openerRect.left + 5);
1498+
});
1499+
});
14241500
});
14251501

14261502
describe("Alignment", () => {

‎packages/main/src/ComboBox.ts‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,6 @@ import "./ComboBoxItemGroup.js";
8787
// eslint-disable-next-line
8888
import { isInstanceOfComboBoxItemGroup } from "./ComboBoxItemGroup.js";
8989
import type ComboBoxFilter from "./types/ComboBoxFilter.js";
90-
import PopoverHorizontalAlign from "./types/PopoverHorizontalAlign.js";
9190
import type Input from "./Input.js";
9291
import type { InputEventDetail } from "./Input.js";
9392
import type InputComposition from "./features/InputComposition.js";
@@ -1479,10 +1478,6 @@ class ComboBox extends UI5Element implements IFormInputElement {
14791478
return !this.valueStateMessage.length && this.hasValueStateText;
14801479
}
14811480

1482-
get _valueStatePopoverHorizontalAlign(): `${PopoverHorizontalAlign}` {
1483-
return this.effectiveDir !== "rtl" ? PopoverHorizontalAlign.Start : PopoverHorizontalAlign.End;
1484-
}
1485-
14861481
/**
14871482
* This method is relevant for sap_horizon theme only
14881483
*/

‎packages/main/src/ComboBoxPopoverTemplate.tsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import Icon from "./Icon.js";
22
import Button from "./Button.js";
33
import List from "./List.js";
44
import Input from "./Input.js";
5+
import PopoverHorizontalAlign from "./types/PopoverHorizontalAlign.js";
56
import Popover from "./Popover.js";
67
import ResponsivePopover from "./ResponsivePopover.js";
78
import BusyIndicator from "./BusyIndicator.js";
@@ -116,7 +117,7 @@ export default function ComboBoxPopoverTemplate(this: ComboBox) {
116117
hideArrow={true}
117118
tabindex={-1}
118119
class="ui5-valuestatemessage-popover"
119-
horizontalAlign={this._valueStatePopoverHorizontalAlign}
120+
horizontalAlign={PopoverHorizontalAlign.Start}
120121
placement="Bottom"
121122
opener={this}
122123
open={this.valueStateOpen}

‎packages/main/src/Input.ts‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ import InputType from "./types/InputType.js";
6262
import type Popover from "./Popover.js";
6363
import type Icon from "./Icon.js";
6464
import type { IIcon } from "./Icon.js";
65-
import type PopoverHorizontalAlign from "./types/PopoverHorizontalAlign.js";
65+
6666
// Templates
6767
import InputTemplate from "./InputTemplate.js";
6868
import { StartsWith } from "./Filters.js";
@@ -1969,10 +1969,6 @@ class Input extends UI5Element implements SuggestionComponent, IFormInputElement
19691969
return "";
19701970
}
19711971

1972-
get _valueStatePopoverHorizontalAlign(): `${PopoverHorizontalAlign}` {
1973-
return this.effectiveDir !== "rtl" ? "Start" : "End";
1974-
}
1975-
19761972
/**
19771973
* This method is relevant for sap_horizon theme only
19781974
*/

‎packages/main/src/InputPopoverTemplate.tsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import alert from "@ui5/webcomponents-icons/dist/alert.js";
66
import sysEnter2 from "@ui5/webcomponents-icons/dist/sys-enter-2.js";
77
import information from "@ui5/webcomponents-icons/dist/information.js";
88

9+
import PopoverHorizontalAlign from "./types/PopoverHorizontalAlign.js";
910
import Popover from "./Popover.js";
1011
import ValueState from "@ui5/webcomponents-base/dist/types/ValueState.js";
1112

@@ -24,7 +25,7 @@ export default function InputPopoverTemplate(this: Input, hooks?: { suggestionsL
2425
class="ui5-valuestatemessage-popover"
2526
placement="Bottom"
2627
tabindex={-1}
27-
horizontalAlign={this._valueStatePopoverHorizontalAlign}
28+
horizontalAlign={PopoverHorizontalAlign.Start}
2829
opener={this}
2930
open={this.valueStateOpen}
3031
onClose={this._handleValueStatePopoverAfterClose}

‎packages/main/src/MenuItem.ts‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ import type { ListItemAccessibilityAttributes } from "./ListItem.js";
3030
import type List from "./List.js";
3131
import ListItem from "./ListItem.js";
3232
import type ResponsivePopover from "./ResponsivePopover.js";
33-
import type PopoverPlacement from "./types/PopoverPlacement.js";
3433
import { isInstanceOfMenuSeparator } from "./MenuSeparator.js";
3534
import { isInstanceOfMenuItemGroup } from "./MenuItemGroup.js";
3635
import MenuItemTemplate from "./MenuItemTemplate.js";
@@ -357,10 +356,6 @@ class MenuItem extends ListItem implements IMenuItem {
357356
}
358357
}
359358

360-
get placement(): `${PopoverPlacement}` {
361-
return this.isRtl ? "Start" : "End";
362-
}
363-
364359
get isRtl() {
365360
return this.effectiveDir === "rtl";
366361
}

‎packages/main/src/MenuItemTemplate.tsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type MenuItem from "./MenuItem.js";
2+
import PopoverPlacement from "./types/PopoverPlacement.js";
23
import ResponsivePopover from "./ResponsivePopover.js";
34
import Button from "./Button.js";
45
import List from "./List.js";
@@ -91,7 +92,7 @@ function listItemPostContent(this: MenuItem) {
9192
preventFocusRestore={true}
9293
hideArrow={true}
9394
allowTargetOverlap={true}
94-
placement={this.placement}
95+
placement={PopoverPlacement.End}
9596
verticalAlign="Top"
9697
accessibleName={this.accessibleNameText}
9798
onBeforeOpen={this._beforePopoverOpen}

‎packages/main/src/MultiComboBox.ts‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,6 @@ import type ComboBoxFilter from "./types/ComboBoxFilter.js";
112112
import CheckBox from "./CheckBox.js";
113113
import Input from "./Input.js";
114114
import type { InputEventDetail } from "./Input.js";
115-
import type PopoverHorizontalAlign from "./types/PopoverHorizontalAlign.js";
116115
import SuggestionItem from "./SuggestionItem.js";
117116
import type InputComposition from "./features/InputComposition.js";
118117

@@ -2199,10 +2198,6 @@ class MultiComboBox extends UI5Element implements IFormInputElement {
21992198
return shouldBeExpanded;
22002199
}
22012200

2202-
get _valueStatePopoverHorizontalAlign(): `${PopoverHorizontalAlign}` {
2203-
return this.effectiveDir !== "rtl" ? "Start" : "End";
2204-
}
2205-
22062201
get iconsCount() {
22072202
const slottedIconsCount = this.icon?.length || 0;
22082203
const clearIconCount = Number(this._effectiveShowClearIcon) ?? 0;

0 commit comments

Comments
 (0)