Skip to content

Commit

Permalink
fix(ld-tooltip): event handlers not called in tooltip
Browse files Browse the repository at this point in the history
  • Loading branch information
borisdiakur committed May 26, 2023
1 parent 978ee53 commit dc80ee0
Show file tree
Hide file tree
Showing 9 changed files with 663 additions and 340 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ permalink: guides/troubleshooting/popped-out-element-is-rendered-in-wrong-contai

## Problem

Some components have a popper element that needs to stay above every other element when popped out. The [ld-select](components/ld-select/) component and the [ld-tooltip](components/ld-tooltip/) component are two examples of such components. By default, this is achieved by rendering the popped-out element as a direct child of the `body` element.
Some components have a popper element that needs to stay above every other element when popped out. The [ld-select](components/ld-select/) component, the [ld-context-menu](components/ld-context-menu/) component and the [ld-tooltip](components/ld-tooltip/) component are examples of such components. By default, this is achieved by rendering the popped-out element as a direct child of the `body` element.

However, in some cases this default behavior can be problematic. For instance:

Expand Down
26 changes: 25 additions & 1 deletion src/liquid/components/ld-context-menu/ld-context-menu.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
import { Component, Element, h, Prop, State, Watch } from '@stencil/core'
import {
Component,
Element,
h,
Method,
Prop,
State,
Watch,
} from '@stencil/core'
import { isHtmlElement } from '../../utils/type-checking'
import { isInnerFocusable } from '../../utils/focus'

Expand All @@ -25,6 +33,9 @@ export class LdContextMenu {
/** Size of the context menu. */
@Prop() size?: 'sm' | 'lg'

/** Tether options object to be merged with the default options (optionally stringified). */
@Prop() tetherOptions?: Partial<Tether.ITetherOptions> | string

@State() initialized = false

private resetFocus = async () => {
Expand Down Expand Up @@ -68,6 +79,18 @@ export class LdContextMenu {
await firstMenuItem.focusInner()
}

/** Show context menu */
@Method()
async showContextMenu() {
await this.tooltipRef.showTooltip()
}

/** Hide context menu */
@Method()
async hideContextMenu() {
await this.tooltipRef.hideTooltip()
}

@Watch('size')
private updateSize() {
if (this.size) {
Expand Down Expand Up @@ -102,6 +125,7 @@ export class LdContextMenu {
position={this.position}
preventScreenreader
tag="span"
tetherOptions={this.tetherOptions}
triggerType="click"
unstyled
>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,14 @@
import { Component, Element, Host, h, Prop, State, Method } from '@stencil/core'
import {
Component,
Element,
Host,
h,
Prop,
State,
Method,
Event,
EventEmitter,
} from '@stencil/core'
import { cloneAttributes } from '../../../utils/cloneAttributes'

type Mode = 'highlight' | 'danger' | 'neutral'
Expand Down Expand Up @@ -38,6 +48,9 @@ export class LdMenuitem implements InnerFocusable {
/** Tab index of the menu item. */
@Prop() ldTabindex?: number

/** Prevent closing of the context menu on click. */
@Prop() preventClose?: boolean

/** Display mode. */
@Prop() mode?: Mode = 'neutral'

Expand All @@ -62,6 +75,17 @@ export class LdMenuitem implements InnerFocusable {
this.buttonRef?.focusInner()
}

/**
* @internal
* Emitted on menu item click if preventClose prop is not truethy.
*/
@Event() ldclosetooltip: EventEmitter

private handleClick = (ev: MouseEvent) => {
if (this.preventClose) return
this.ldclosetooltip.emit(ev)
}

componentWillLoad() {
this.attributesObserver = cloneAttributes.call(this, [
'ld-tabindex',
Expand All @@ -88,6 +112,7 @@ export class LdMenuitem implements InnerFocusable {
justifyContent="start"
ldTabindex={this.ldTabindex}
mode={modeMap.get(this.mode)}
onClick={this.handleClick}
part="focusable button"
ref={(element) => (this.buttonRef = element)}
size={this.size}
Expand Down
19 changes: 10 additions & 9 deletions src/liquid/components/ld-context-menu/ld-menuitem/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -121,15 +121,16 @@ The `ld-menuitem` component is a subcomponent for `ld-context-menu` / `ld-menu`

## Properties

| Property | Attribute | Description | Type | Default |
| ------------ | ------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------- | ----------- |
| `disabled` | `disabled` | Disabled state of the menu item. | `boolean` | `undefined` |
| `href` | `href` | Transforms the menu item to an anchor element. See [mdn docs](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a#attr-href) for more information on the `href` attribute. | `string` | `undefined` |
| `key` | `key` | for tracking the node's identity when working with lists | `string \| number` | `undefined` |
| `ldTabindex` | `ld-tabindex` | Tab index of the menu item. | `number` | `undefined` |
| `mode` | `mode` | Display mode. | `"danger" \| "highlight" \| "neutral"` | `'neutral'` |
| `ref` | `ref` | reference to component | `any` | `undefined` |
| `target` | `target` | The `target` attributed can be used in conjunction with the `href` attribute. See [mdn docs](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a#attr-target) for more information on the `target` attribute. | `"_blank" \| "_parent" \| "_self" \| "_top"` | `undefined` |
| Property | Attribute | Description | Type | Default |
| -------------- | --------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------- | ----------- |
| `disabled` | `disabled` | Disabled state of the menu item. | `boolean` | `undefined` |
| `href` | `href` | Transforms the menu item to an anchor element. See [mdn docs](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a#attr-href) for more information on the `href` attribute. | `string` | `undefined` |
| `key` | `key` | for tracking the node's identity when working with lists | `string \| number` | `undefined` |
| `ldTabindex` | `ld-tabindex` | Tab index of the menu item. | `number` | `undefined` |
| `mode` | `mode` | Display mode. | `"danger" \| "highlight" \| "neutral"` | `'neutral'` |
| `preventClose` | `prevent-close` | Prevent closing of the context menu on click. | `boolean` | `undefined` |
| `ref` | `ref` | reference to component | `any` | `undefined` |
| `target` | `target` | The `target` attributed can be used in conjunction with the `href` attribute. See [mdn docs](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a#attr-target) for more information on the `target` attribute. | `"_blank" \| "_parent" \| "_self" \| "_top"` | `undefined` |


## Methods
Expand Down
62 changes: 56 additions & 6 deletions src/liquid/components/ld-context-menu/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ permalink: components/ld-context-menu/

# ld-context-menu

The context menu is a pop-up menu that appears upon user interaction. It offers a limited set of choices that are available in the current state, or context.

<ld-notice headline="Note" mode="warning">
If your application is mounted to a different element than the <code>body</code> element, or you have <em>z-order</em> related issues, you may need to configure the <code>bodyElement</code> option using the <code>tetherOptions</code> property. For more details check out the <a href="guides/troubleshooting/#popped-out-element-is-rendered-in-wrong-container">related troubleshooting section</a>.
</ld-notice>

## Examples

<ld-notice mode="warning">
Expand Down Expand Up @@ -185,17 +191,61 @@ permalink: components/ld-context-menu/
The <code>position</code> prop is identical to the one of the <code>ld-tooltip</code> component. Please refer to the <a href="components/ld-tooltip/#positioning"><code>ld-tooltip</code>-documentation</a> for a full example of all available positions.
</ld-notice>

### Prevent auto-closing

You can prevent auto-closing of the context menu on click, by attaching the `prevent-close` attribute to the `ld-menuitem` component.

{% example %}
<ld-context-menu>
<ld-button slot="trigger">Open menu</ld-button>
<ld-menuitem>Will close</ld-menuitem>
<ld-menuitem prevent-close>Won&apos;t close</ld-menuitem>
</ld-context-menu>

<!-- React component -->

<LdContextMenu>
<LdButton slot="trigger">Open menu</LdButton>
<LdMenuitem>Will close</LdMenuitem>
<LdMenuitem preventClose>Won&apos;t close</LdMenuitem>
</LdContextMenu>
{% endexample %}

<!-- Auto Generated Below -->


## Properties

| Property | Attribute | Description | Type | Default |
| ---------- | ---------- | ------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------- |
| `key` | `key` | for tracking the node's identity when working with lists | `string \| number` | `undefined` |
| `position` | `position` | Position of the context menu relative to the trigger element. | `"bottom center" \| "bottom left" \| "bottom right" \| "left bottom" \| "left middle" \| "left top" \| "right bottom" \| "right middle" \| "right top" \| "top center" \| "top left" \| "top right"` | `'bottom left'` |
| `ref` | `ref` | reference to component | `any` | `undefined` |
| `size` | `size` | Size of the context menu. | `"lg" \| "sm"` | `undefined` |
| Property | Attribute | Description | Type | Default |
| --------------- | ---------------- | ------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | --------------- |
| `key` | `key` | for tracking the node's identity when working with lists | `string \| number` | `undefined` |
| `position` | `position` | Position of the context menu relative to the trigger element. | `"bottom center" \| "bottom left" \| "bottom right" \| "left bottom" \| "left middle" \| "left top" \| "right bottom" \| "right middle" \| "right top" \| "top center" \| "top left" \| "top right"` | `'bottom left'` |
| `ref` | `ref` | reference to component | `any` | `undefined` |
| `size` | `size` | Size of the context menu. | `"lg" \| "sm"` | `undefined` |
| `tetherOptions` | `tether-options` | Tether options object to be merged with the default options (optionally stringified). | `string \| { attachment?: string; bodyElement?: HTMLElement; classes?: { [className: string]: string \| boolean; }; classPrefix?: string; constraints?: ITetherConstraint[]; element?: any; enabled?: boolean; offset?: string; optimizations?: any; target?: any; targetAttachment?: string; targetOffset?: string; targetModifier?: string; }` | `undefined` |


## Methods

### `hideContextMenu() => Promise<void>`

Hide context menu

#### Returns

Type: `Promise<void>`



### `showContextMenu() => Promise<void>`

Show context menu

#### Returns

Type: `Promise<void>`




## Shadow Parts
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,85 @@ describe('ld-context-menu', () => {
expect(ldButton.focusInner).toHaveBeenCalled()
})

it('allows to open and close menu via methods', async () => {
const page = await newSpecPage({
components: [LdContextMenu, LdMenuitem, LdTooltip, LdMenu, LdButton],
template: () => (
<ld-context-menu>
<ld-button slot="trigger">Open</ld-button>
<ld-menuitem>Menu item</ld-menuitem>
</ld-context-menu>
),
})

const contextMenu = page.root as HTMLLdContextMenuElement
const tooltip = contextMenu.shadowRoot.querySelector('ld-tooltip')
const menu = page.root.shadowRoot.querySelector('ld-menu')
const triggerButton = page.root.querySelector('ld-button')
const tooltipTrigger = tooltip.shadowRoot.querySelector(
'.ld-tooltip__trigger'
)
const popper = tooltip.shadowRoot.querySelector('ld-tooltip-popper')

await prepareAndGetMenuInTooltip(page, [triggerButton], [menu])

expect(popper).toHaveAttribute('aria-hidden')

await contextMenu.showContextMenu()
await page.waitForChanges()

expect(tooltipTrigger).toHaveClass('ld-tether-enabled')
expect(popper).not.toHaveAttribute('aria-hidden')

await contextMenu.hideContextMenu()
await page.waitForChanges()

expect(tooltip.hideTooltip).toHaveBeenCalled()
})

it('prevents closing menu', async () => {
const page = await newSpecPage({
components: [LdContextMenu, LdMenuitem, LdTooltip, LdMenu, LdButton],
template: () => (
<ld-context-menu>
<ld-button slot="trigger">Open</ld-button>
<ld-menuitem prevent-close>Can not close</ld-menuitem>
<ld-menuitem>Can close</ld-menuitem>
</ld-context-menu>
),
})

const contextMenu = page.root as HTMLLdContextMenuElement
const tooltip = page.root.shadowRoot.querySelector('ld-tooltip')
const menu = page.root.shadowRoot.querySelector('ld-menu')
const triggerButton = page.root.querySelector('ld-button')
const popper = tooltip.shadowRoot.querySelector('ld-tooltip-popper')

const menuInTooltip = await prepareAndGetMenuInTooltip(
page,
[triggerButton],
[menu]
)
const menuItemsInTooltip = Array.from(
menuInTooltip.querySelectorAll('ld-menuitem')
)

await contextMenu.showContextMenu()
await page.waitForChanges()

expect(popper).not.toHaveAttribute('aria-hidden')

menuItemsInTooltip[0].click()
await page.waitForChanges()

expect(tooltip.hideTooltip).not.toHaveBeenCalled()

menuItemsInTooltip[1].click()
await page.waitForChanges()

expect(tooltip.hideTooltip).not.toHaveBeenCalled()
})

it('does not throw when trying to set inner focus before hydration', async () => {
const component = new LdMenuitem()
await component.focusInner()
Expand Down
Loading

1 comment on commit dc80ee0

@vercel
Copy link

@vercel vercel bot commented on dc80ee0 May 26, 2023

Choose a reason for hiding this comment

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

Successfully deployed to the following URLs:

liquid – ./

liquid-oxygen.vercel.app
liquid-uxsd.vercel.app
liquid-git-main-uxsd.vercel.app

Please sign in to comment.