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.

Sign up for GitHub

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

fix(masthead): Esc to close Cloud Masthead #10555

Merged
merged 7 commits into from
Jun 15, 2023
Merged

fix(masthead): Esc to close Cloud Masthead #10555

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b106bee
10174: borrow code from megamenu to ensure plain menu closes on esc w…
pjudge Jun 13, 2023
85cc2b9
Merge branch 'main' into fix/cloud-masthead-esc-to-close-10174
pjudge Jun 13, 2023
429fc8e
10174: fix esc issue upstream
pjudge Jun 13, 2023
a13cba8
10174: running yarn format and ci-check
pjudge Jun 13, 2023
45af8d1
10174: updates from code review
pjudge Jun 14, 2023
6c9682d
10174: updates from code reivew - revert an unneeded change
pjudge Jun 14, 2023
b5b5b23
Merge branch 'main' into fix/cloud-masthead-esc-to-close-10174
pjudge Jun 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions packages/carbon-web-components/src/components/ui-shell/header-menu.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ const { prefix } = settings;
* Header menu.
*
* @element bx-header-menu
* @csspart trigger The trigger button.
* @csspart topMenuItem The trigger button.
pjudge marked this conversation as resolved.
Show resolved Hide resolved
* @csspart trigger-icon The trigger button icon.
* @csspart menu-body The menu body.
*/
Expand All @@ -33,8 +33,8 @@ class BXHeaderMenu extends HostListenerMixin(FocusMixin(LitElement)) {
/**
* The trigger button.
*/
@query('a')
private _trigger!: HTMLElement;
@query('[part="trigger"]')
protected _topMenuItem!: HTMLElement;

/**
* Handles `click` event handler on this element.
Expand All @@ -46,6 +46,8 @@ class BXHeaderMenu extends HostListenerMixin(FocusMixin(LitElement)) {
/**
* Handler for the `keydown` event on the trigger button.
*/
@HostListener('keydown')
// @ts-ignore: The decorator refers to this method but TS thinks this method is not referred to
private _handleKeydownTrigger({ key }: KeyboardEvent) {
if (key === 'Esc' || key === 'Escape') {
this._handleUserInitiatedToggle(false);
Expand All @@ -60,7 +62,7 @@ class BXHeaderMenu extends HostListenerMixin(FocusMixin(LitElement)) {
private _handleUserInitiatedToggle(force: boolean = !this.expanded) {
this.expanded = force;
if (!force) {
this._trigger.focus();
this._topMenuItem.focus();
}
}

Expand Down Expand Up @@ -123,7 +125,6 @@ class BXHeaderMenu extends HostListenerMixin(FocusMixin(LitElement)) {
triggerContent,
menuLabel,
_handleClick: handleClick,
_handleKeydownTrigger: handleKeydownTrigger,
} = this;
return html`
<a
Expand All @@ -134,8 +135,7 @@ class BXHeaderMenu extends HostListenerMixin(FocusMixin(LitElement)) {
href="javascript:void 0"
aria-haspopup="menu"
aria-expanded="${String(Boolean(expanded))}"
@click=${handleClick}
@keydown=${handleKeydownTrigger}>
@click=${handleClick}>
${triggerContent}${ChevronDownGlyph({
part: 'trigger-icon',
class: `${prefix}--header__menu-arrow`,
Expand Down
17 changes: 0 additions & 17 deletions packages/web-components/src/components/masthead/megamenu-top-nav-menu.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,6 @@ class DDSMegaMenuTopNavMenu extends DDSTopNavMenu {
@query(`.${prefix}--header__menu`)
private _menuNode!: HTMLElement;

/**
* The trigger button.
*/
@query('[part="trigger"]')
private _topMenuItem!: HTMLAnchorElement;

/**
* scrollbar width.
*/
Expand All @@ -54,16 +48,6 @@ class DDSMegaMenuTopNavMenu extends DDSTopNavMenu {
this.ownerDocument!.defaultView!.innerWidth -
this.ownerDocument!.body.offsetWidth;

/**
* Removes inherited _handleBlur method from BXHeaderMenu
*/
private _handleKeydown = (event: KeyboardEvent) => {
if (event.key === 'Escape') {
this.expanded = false;
this._topMenuItem.focus();
}
};

/**
* The observer for the resize of the viewport.
*/
Expand Down Expand Up @@ -127,7 +111,6 @@ class DDSMegaMenuTopNavMenu extends DDSTopNavMenu {
connectedCallback() {
super.connectedCallback();
this._cleanAndCreateObserverResize({ create: true });
this.addEventListener('keydown', this._handleKeydown);
}

disconnectedCallback() {
Expand Down
10 changes: 2 additions & 8 deletions packages/web-components/src/components/masthead/top-nav-menu.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
* LICENSE file in the root directory of this source tree.
*/

import { property, query } from 'lit-element';
import { property } from 'lit-element';
import BXHeaderMenu from '../../internal/vendor/@carbon/web-components/components/ui-shell/header-menu.js';
import ddsSettings from '../../internal/vendor/@carbon/ibmdotcom-utilities/utilities/settings/settings';
import styles from './masthead.scss';
Expand All @@ -22,12 +22,6 @@ const { stablePrefix: ddsPrefix } = ddsSettings;
*/
@customElement(`${ddsPrefix}-top-nav-menu`)
class DDSTopNavMenu extends BXHeaderMenu {
/**
* The trigger button.
*/
@query('[part="trigger"]')
private _triggerNode?: HTMLAnchorElement;

/**
* `true` if this submenu should be in its active state.
*/
Expand All @@ -37,7 +31,7 @@ class DDSTopNavMenu extends BXHeaderMenu {
updated(changedProperties) {
super.updated(changedProperties);
if (changedProperties.has('active')) {
const { active, _triggerNode: triggerNode } = this;
const { active, _topMenuItem: triggerNode } = this;
if (triggerNode) {
triggerNode.setAttribute('data-selected', String(Boolean(active)));
}
Expand Down