Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add horizontalAlignment to Menu.open() options #732

Merged
merged 13 commits into from
Dec 19, 2024
18 changes: 17 additions & 1 deletion packages/widgets/src/menu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -465,9 +465,12 @@ export class Menu extends Widget {
let forceY = options.forceY || false;
const host = options.host ?? null;
const ref = options.ref ?? null;
const align =
options.align ??
(document.documentElement.dir === 'rtl' ? 'right' : 'left');

// Open the menu as a root menu.
Private.openRootMenu(this, x, y, forceX, forceY, host, ref);
Private.openRootMenu(this, x, y, forceX, forceY, align, host, ref);

// Activate the menu to accept keyboard input.
this.activate();
Expand Down Expand Up @@ -1009,6 +1012,13 @@ export namespace Menu {
* menu to be added as the last child of the host.
*/
ref?: HTMLElement;

/**
* The alignment of the menu.
*
* The default is `'left'` unless the document `dir` attribute is `'rtl'`
*/
align?: 'left' | 'right';
Copy link
Member

Choose a reason for hiding this comment

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

To make this more future-proof, should we call it horizontalAlignment instead? So that we can add verticalAlignment if we need it later?

Otherwise it looks great!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point! I just updated that.

}

/**
Expand Down Expand Up @@ -1559,6 +1569,7 @@ namespace Private {
y: number,
forceX: boolean,
forceY: boolean,
align: 'left' | 'right',
host: HTMLElement | null,
ref: HTMLElement | null
): void {
Expand Down Expand Up @@ -1589,6 +1600,11 @@ namespace Private {
// Measure the size of the menu.
let { width, height } = node.getBoundingClientRect();

// align the menu to the right of the target if requested or language is RTL
if (align === 'right') {
x -= width;
}

// Adjust the X position of the menu to fit on-screen.
if (!forceX && x + width > px + cw) {
x = px + cw - width;
Expand Down
24 changes: 24 additions & 0 deletions packages/widgets/tests/src/menu.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -604,6 +604,30 @@ describe('@lumino/widgets', () => {
);
});

it('should accept align menu flags', () => {
menu.addItem({ command: 'test' });
menu.open(300, 300, { align: 'right' });
let { width } = menu.node.getBoundingClientRect();
const expectedX = Math.floor(300 - width);
expect(
menu.node.style.transform.startsWith(`translate(${expectedX}`)
).to.equal(true);
expect(menu.node.style.transform.endsWith('px, 300px)')).to.equal(true);
});

it.only('align should default to right if language direction is rtl', () => {
document.documentElement.setAttribute('dir', 'rtl');
menu.addItem({ command: 'test' });
menu.open(300, 300);
let { width } = menu.node.getBoundingClientRect();
const expectedX = Math.floor(300 - width);
expect(
menu.node.style.transform.startsWith(`translate(${expectedX}`)
).to.equal(true);
expect(menu.node.style.transform.endsWith('px, 300px)')).to.equal(true);
document.documentElement.removeAttribute('dir'); // Reset the direction
});

it('should bail if already attached', () => {
menu.addItem({ command: 'test' });
menu.open(10, 10);
Expand Down
1 change: 1 addition & 0 deletions review/api/widgets.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -739,6 +739,7 @@ export namespace Menu {
type?: ItemType;
}
export interface IOpenOptions {
align?: 'left' | 'right';
forceX?: boolean;
forceY?: boolean;
host?: HTMLElement;
Expand Down
Loading