Skip to content

Commit

Permalink
fix(menu): remove Menu DOM node
Browse files Browse the repository at this point in the history
This change brings the Ember implementation closer inline with the React/Vue implementations, which do not create a DOM node for this component automatically. This makes working with the component easier, as it no longer introduces un-necessary DOM nodes.

Note: the React and Vue implementations allow for optionally creating a DOM node for the `Menu` component. This is possible in Ember, technically, but probably more trouble than it's worth, since the user of `Menu` can just wrap the component in an additional DOM node themselves if that's what they want. I believe the React and Vue implementations include this functionality for consistency, so that _all_ Headless UI components can optionally render an alternate DOM node, but those frameworks have an easier ability to pull this off than Ember does.

A focus trap has been introduced to power the click-outside-to-dismiss functionality, which also resolves the issue of focus needing to be locked in the `Menu::Items` component when rendered, which was previously not the case.

BREAKING CHANGE: If you were previously depending on `Menu` rendering a DOM node, you'll want to wrap your usage of `Menu` with a `div` and move any attributes or modifiers to that element instead.
  • Loading branch information
alexlafroscia committed Jun 28, 2021
1 parent fa2ca27 commit e46741d
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 60 deletions.
72 changes: 32 additions & 40 deletions addon/components/menu.hbs
Original file line number Diff line number Diff line change
@@ -1,42 +1,34 @@
<div
id={{this.guid}}
...attributes
{{on 'pointerdown' this.onPointerDown}}
{{click-outside this.close event='mouseup'}}
>
{{yield
(hash
{{yield
(hash
isOpen=this.isOpen
Button=(component
'menu/button'
buttonGuid=this.buttonGuid
itemsGuid=this.itemsGuid
isOpen=this.isOpen
Button=(component
'menu/button'
guid=this.buttonGuid
menuGuid=this.guid
itemsGuid=this.itemsGuid
isOpen=this.isOpen
onClick=this.toggle
openMenu=this.open
closeMenu=this.close
goToFirstItem=this.goToFirstItem
goToLastItem=this.goToLastItem
goToNextItem=this.goToNextItem
goToPreviousItem=this.goToPreviousItem
)
Items=(component
'menu/items'
guid=this.itemsGuid
buttonGuid=this.buttonGuid
isOpen=this.isOpen
closeMenu=this.close
activeItem=this.activeItem
registerItem=this.registerItem
unregisterItem=this.unregisterItem
goToFirstItem=this.goToFirstItem
goToLastItem=this.goToLastItem
goToNextItem=this.goToNextItem
goToPreviousItem=this.goToPreviousItem
goToItem=this.goToItem
search=(perform this.searchTask)
)
openMenu=this.open
closeMenu=this.close
toggleMenu=this.toggle
goToFirstItem=this.goToFirstItem
goToLastItem=this.goToLastItem
goToNextItem=this.goToNextItem
goToPreviousItem=this.goToPreviousItem
)
}}
</div>
Items=(component
'menu/items'
buttonGuid=this.buttonGuid
itemsGuid=this.itemsGuid
isOpen=this.isOpen
closeMenu=this.close
activeItem=this.activeItem
registerItem=this.registerItem
unregisterItem=this.unregisterItem
goToFirstItem=this.goToFirstItem
goToLastItem=this.goToLastItem
goToNextItem=this.goToNextItem
goToPreviousItem=this.goToPreviousItem
goToItem=this.goToItem
search=(perform this.searchTask)
)
)
}}
18 changes: 1 addition & 17 deletions addon/components/menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { tracked } from '@glimmer/tracking';
import { action } from '@ember/object';
import { restartableTask, timeout } from 'ember-concurrency';
import { guidFor } from '@ember/object/internals';
import { next } from '@ember/runloop';

export default class Menu extends Component {
guid = `${guidFor(this)}-tailwindui-menu`;
Expand Down Expand Up @@ -32,12 +31,7 @@ export default class Menu extends Component {

@action
close() {
if (this.isOpen) {
this.isOpen = false;
next(() => {
this.buttonElement && this.buttonElement.focus();
});
}
this.isOpen = false;
}

@action
Expand Down Expand Up @@ -124,16 +118,6 @@ export default class Menu extends Component {
this.items = items;
}

@action
onPointerDown(event) {
if (event.defaultPrevented) return;
if (!this.isOpen) return;

next(() => {
this.buttonElement.focus();
});
}

_setActiveItem(item) {
if (item) {
this.activeItem = item;
Expand Down
4 changes: 2 additions & 2 deletions addon/components/menu/button.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
aria-haspopup={{true}}
aria-controls={{@itemsGuid}}
aria-expanded={{@isOpen}}
id='{{@menuGuid}}-button'
id={{@buttonGuid}}
...attributes
{{on 'click' @onClick}}
{{on 'click' @toggleMenu}}
{{on 'keydown' this.onKeydown}}
>
{{yield}}
Expand Down
11 changes: 10 additions & 1 deletion addon/components/menu/items.hbs
Original file line number Diff line number Diff line change
@@ -1,12 +1,21 @@
{{#if @isOpen}}
<div
id={{@guid}}
id={{@itemsGuid}}
aria-labelledby={{@buttonGuid}}
aria-activedescendant={{@activeItem.element.id}}
tabindex='-1'
role='menu'
...attributes
{{on 'keydown' this.onKeydown}}
{{focus-trap
focusTrapOptions=(hash
allowOutsideClick=this.allowClickOutsideFocusTrap
clickOutsideDeactivates=this.clickOutsideFocusTrapDeactivates
fallbackFocus=this.menuItemsElement
initialFocus=this.menuItemsElement
onDeactivate=@closeMenu
)
}}
>
{{yield
(hash
Expand Down
21 changes: 21 additions & 0 deletions addon/components/menu/items.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ import Component from '@glimmer/component';
import { action } from '@ember/object';

export default class Items extends Component {
get menuItemsElement() {
return document.getElementById(this.args.itemsGuid);
}

@action
onKeydown(event) {
switch (event.key) {
Expand All @@ -22,4 +26,21 @@ export default class Items extends Component {
}
}
}

clickInsideMenuTrigger(event) {
const buttonElement = document.getElementById(this.args.buttonGuid);

// The `buttonElement` could not exist if the element has been removed from the DOM
return buttonElement ? buttonElement.contains(event.target) : false;
}

@action
allowClickOutsideFocusTrap(event) {
return this.clickInsideMenuTrigger(event);
}

@action
clickOutsideFocusTrapDeactivates(event) {
return !this.clickInsideMenuTrigger(event);
}
}

0 comments on commit e46741d

Please sign in to comment.