Skip to content

Commit

Permalink
fix: correctly position the menu when the document has scrolled
Browse files Browse the repository at this point in the history
  • Loading branch information
arnog committed Dec 4, 2023
1 parent a91a040 commit 352b55e
Show file tree
Hide file tree
Showing 10 changed files with 58 additions and 41 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@

- Added `mf.showMenu()` method to programmatically show the context menu.

### Bugs Fixed

- Correctly position the menu when the document has been scrolled.

## 0.98.0 (2023-12-03)

Expand Down
2 changes: 1 addition & 1 deletion src/editor-mathfield/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ function findElementWithCaret(element: Element): Element | null {
}

/**
* Return the (x,y) client coordinates of the caret
* Return the (x,y) client coordinates of the caret in viewport coordinates
*/
export function getCaretPoint(
element: Element
Expand Down
2 changes: 1 addition & 1 deletion src/editor/keyboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ export function delegateKeyboardEvents(
return;
}
// If the scrim is up, ignore blur (while the variants panel is up)
const scrimState = Scrim.scrim?.state;
const scrimState = Scrim.state;
if (scrimState === 'open' || scrimState === 'opening') {
event.preventDefault();
event.stopPropagation();
Expand Down
5 changes: 3 additions & 2 deletions src/public/mathfield-element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1804,9 +1804,10 @@ import 'https://unpkg.com/@cortex-js/compute-engine?module';

/** @internal */
handleEvent(evt: Event): void {
// If the scrim is open (variant panel), ignore events
// If the scrim for the variant panel or the menu is
// open, ignore events.
// Otherwise we may end up disconecting from the VK
if (Scrim.scrim && Scrim.scrim.state !== 'closed') return;
if (Scrim.state !== 'closed') return;

// Also, if the menu is open
if (this._mathfield?.menu?.state !== 'closed') return;
Expand Down
4 changes: 2 additions & 2 deletions src/ui/menu/context-menu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,10 @@ export async function onContextMenu(
// If no items visible, don't show anything
if (!menu.visible) return false;

const pt = eventLocation(event);
const location = eventLocation(event);
if (await onLongPress(event)) {
if (menu.state !== 'closed') return false;
menu.show({ target: target, location: pt });
menu.show({ target, location });
}
return true;
}
Expand Down
18 changes: 13 additions & 5 deletions src/ui/menu/menu-list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -257,8 +257,8 @@ export class _MenuListState implements MenuListState {
if (value) value.active = true;
}

if (value) value.element?.focus();
else this._element?.focus();
if (value) value.element?.focus({ preventScroll: true });
else this._element?.focus({ preventScroll: true });
}

/** First activable menu item */
Expand Down Expand Up @@ -380,14 +380,22 @@ export class _MenuListState implements MenuListState {

if (options.location) {
fitInViewport(this.element, {
location: options.location,
alternateLocation: options.alternateLocation,
location: {
x: options.location.x + window.scrollX,
y: options.location.y + window.scrollY,
},
alternateLocation: options.alternateLocation
? {
x: options.alternateLocation.x + window.scrollX,
y: options.alternateLocation.y + window.scrollY,
}
: options.alternateLocation,
verticalPos: 'bottom',
horizontalPos: 'start',
});
}

this.element.focus();
this.element.focus({ preventScroll: true });

// Notify our parent we have opened
// (so the parent can close any other open submenu and/or
Expand Down
10 changes: 4 additions & 6 deletions src/ui/menu/menu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ export class Menu extends _MenuListState implements RootMenuState {

private typingBufferResetTimer = 0;
private typingBuffer: string;
private readonly _scrim: Scrim;
private _openTimestamp?: number;
private _onDismiss?: () => void;
private hysteresisTimer = 0;
Expand All @@ -76,8 +75,6 @@ export class Menu extends _MenuListState implements RootMenuState {
};
this.typingBuffer = '';
this.state = 'closed';

this._scrim = new Scrim({ onClose: () => this.hide() });
}

get modifiers(): KeyboardModifiers {
Expand Down Expand Up @@ -256,7 +253,7 @@ export class Menu extends _MenuListState implements RootMenuState {
}

get scrim(): Element {
return this._scrim.element;
return Scrim.element;
}

private connectScrim(target?: Node | null): void {
Expand All @@ -269,7 +266,7 @@ export class Menu extends _MenuListState implements RootMenuState {
scrim.addEventListener('keyup', this);
scrim.addEventListener('pointermove', this);

this._scrim.open({ root: target });
Scrim.open({ root: target });
}

private disconnectScrim(): void {
Expand All @@ -281,14 +278,15 @@ export class Menu extends _MenuListState implements RootMenuState {
scrim.removeEventListener('keydown', this);
scrim.removeEventListener('keyup', this);
scrim.removeEventListener('pointermove', this);
if (this._scrim.state === 'open') this._scrim.close();
if (Scrim.state === 'open') Scrim.scrim.close();
}

get rootMenu(): Menu {
// I AM THE ONE WHO KNOCKS
return this;
}

/** Locations are in viewport coordinate */
show(options?: {
target?: Node | null; // Where the menu should attach
location?: { x: number; y: number };
Expand Down
48 changes: 27 additions & 21 deletions src/ui/utils/scrim.ts
Original file line number Diff line number Diff line change
@@ -1,37 +1,45 @@
import { deepActiveElement } from 'ui/events/utils';

export class Scrim {
static scrim: Scrim;
private static _scrim: Scrim | undefined;
static get scrim(): Scrim {
if (!Scrim._scrim) Scrim._scrim = new Scrim();
return Scrim._scrim;
}
static open(options: { root?: Node | null; child?: HTMLElement }): void {
Scrim.scrim.open(options);
}
static close(): void {
Scrim.scrim.close();
}
static get state(): 'closed' | 'opening' | 'open' | 'closing' {
return Scrim.scrim.state;
}
static get element(): HTMLElement {
return Scrim.scrim.element;
}

private _element?: HTMLElement;
state: 'closed' | 'opening' | 'open' | 'closing';

private readonly preventOverlayClose: boolean;
private readonly onClose?: () => void;
private readonly lightDismiss: boolean;
private readonly translucent: boolean;

private _element?: HTMLElement;

private savedOverflow?: string;
private savedMarginRight?: string;

private savedActiveElement?: HTMLOrSVGElement | null;

state: 'closed' | 'opening' | 'open' | 'closing';

private readonly translucent: boolean;

/**
* - If `options.preventOverlayClose` is false, the scrim is closed if the
* - If `lightDismiss` is true, the scrim is closed if the
* user clicks on the scrim. That's the behavior for menus, for example.
* When you need a fully modal situation until the user has made an
* explicit choice (validating cookie usage, for example), set
* `preventOverlayClose` to true.
* - `onClose()` is called when the scrim is being closed
* -
* `lightDismiss` to fallse.
*/
constructor(options?: {
translucent?: boolean;
preventOverlayClose?: boolean;
onClose?: () => void;
}) {
this.preventOverlayClose = options?.preventOverlayClose ?? false;
constructor(options?: { translucent?: boolean; lightDismiss?: boolean }) {
this.lightDismiss = options?.lightDismiss ?? true;
this.translucent = options?.translucent ?? false;

this.state = 'closed';
Expand Down Expand Up @@ -102,8 +110,6 @@ export class Scrim {
}
this.state = 'closing';

if (typeof this.onClose === 'function') this.onClose();

const { element } = this;
element.removeEventListener('click', this);
document.removeEventListener('touchmove', this, false);
Expand All @@ -125,7 +131,7 @@ export class Scrim {
}

handleEvent(ev: Event): void {
if (!this.preventOverlayClose) {
if (this.lightDismiss) {
if (ev.target === this._element && ev.type === 'click') {
this.close();
ev.preventDefault();
Expand Down
5 changes: 2 additions & 3 deletions src/virtual-keyboard/variants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,7 @@ export function showVariantsPanel(
//
// Create the scrim and attach the variants panel to it
//
if (!Scrim.scrim) Scrim.scrim = new Scrim();
Scrim.scrim.open({
Scrim.open({
root: keyboard?.container?.querySelector('.ML__keyboard'),
child: variantPanel,
});
Expand Down Expand Up @@ -269,7 +268,7 @@ export function showVariantsPanel(
export function hideVariantsPanel(): void {
gVariantPanelController?.abort();
gVariantPanelController = null;
Scrim.scrim?.close();
Scrim.close();
}

function makeVariants(
Expand Down
2 changes: 2 additions & 0 deletions test/smoke/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@ <h2>Latex to Speakable Text</h2>
<button id="grow">Grow</button>
<button id="shrink">Shrink</button>
</div>

<div style="height: 800px"></div>
</main>

<script type="module">
Expand Down

0 comments on commit 352b55e

Please sign in to comment.