From 09cb6da8fb0335494df04f8310242d3248507b40 Mon Sep 17 00:00:00 2001 From: Elliott Marquez Date: Fri, 4 Aug 2023 13:18:02 -0700 Subject: [PATCH] refactor(list,menu)!: add href to (list|menu)-item and remove (menu|list)-item-link BREAKING CHANGE: list-item-link and menu-item-link have been removed and their functionality has been added to menu-item and list-item respectively. PiperOrigin-RevId: 553894393 --- all.ts | 4 -- catalog/site/_includes/default.html | 4 +- catalog/site/css/global.css | 10 +-- .../src/hydration-entrypoints/navigation.ts | 2 +- catalog/src/ssr.ts | 2 +- common.ts | 4 -- list/demo/demo.ts | 9 +-- list/demo/stories.ts | 7 +- list/internal/listitem/list-item.ts | 33 +++++++-- .../listitemlink/list-item-link-only.ts | 34 --------- list/internal/listitemlink/list-item-link.ts | 47 ------------ list/list-item-link.ts | 61 ---------------- list/list_test.ts | 72 ++++++++----------- menu/demo/demo.ts | 3 - menu/demo/stories.ts | 11 ++- menu/internal/menuitemlink/menu-item-link.ts | 48 ------------- menu/menu-item-link.ts | 47 ------------ 17 files changed, 74 insertions(+), 324 deletions(-) delete mode 100644 list/internal/listitemlink/list-item-link-only.ts delete mode 100644 list/internal/listitemlink/list-item-link.ts delete mode 100644 list/list-item-link.ts delete mode 100644 menu/internal/menuitemlink/menu-item-link.ts delete mode 100644 menu/menu-item-link.ts diff --git a/all.ts b/all.ts index edde6080c..88e1ea331 100644 --- a/all.ts +++ b/all.ts @@ -38,10 +38,8 @@ import './iconbutton/icon-button.js'; import './iconbutton/outlined-icon-button.js'; import './list/list.js'; import './list/list-item.js'; -import './list/list-item-link.js'; import './menu/menu.js'; import './menu/menu-item.js'; -import './menu/menu-item-link.js'; import './menu/sub-menu-item.js'; import './progress/circular-progress.js'; import './progress/linear-progress.js'; @@ -86,10 +84,8 @@ export * from './iconbutton/icon-button.js'; export * from './iconbutton/outlined-icon-button.js'; export * from './list/list.js'; export * from './list/list-item.js'; -export * from './list/list-item-link.js'; export * from './menu/menu.js'; export * from './menu/menu-item.js'; -export * from './menu/menu-item-link.js'; export * from './menu/sub-menu-item.js'; export * from './progress/circular-progress.js'; export * from './progress/linear-progress.js'; diff --git a/catalog/site/_includes/default.html b/catalog/site/_includes/default.html index 0091c961a..9d86d8e85 100644 --- a/catalog/site/_includes/default.html +++ b/catalog/site/_includes/default.html @@ -59,13 +59,13 @@ {% for component in collections.component|filtersort('data.name') %}
  • - + >
    • {% endfor %}     diff --git a/catalog/site/css/global.css b/catalog/site/css/global.css index 28b347c57..d81502067 100644 --- a/catalog/site/css/global.css +++ b/catalog/site/css/global.css @@ -85,18 +85,18 @@ a:focus-visible { text-decoration: underline; } -nav-drawer md-list-item-link[selected] { +nav-drawer md-list-item[href][selected] { --md-list-item-list-item-container-color: var( --md-sys-color-surface-container-highest ); } @media (forced-colors: active) { - nav-drawer md-list-item-link[selected] { + nav-drawer md-list-item[href][selected] { border: 4px double CanvasText; } - nav-drawer md-list-item-link { + nav-drawer md-list-item[href] { border-radius: var(--catalog-shape-xl); border: 1px solid CanvasText; } @@ -110,12 +110,12 @@ md-list { min-width: unset; } -md-list-item-link { +md-list-item[href] { margin-block: var(--catalog-spacing-m); display: block; } -md-list-item-link:first-of-type { +md-list-item[href]:first-of-type { margin-block: 4px; } diff --git a/catalog/src/hydration-entrypoints/navigation.ts b/catalog/src/hydration-entrypoints/navigation.ts index df016e3d7..aab6bcc66 100644 --- a/catalog/src/hydration-entrypoints/navigation.ts +++ b/catalog/src/hydration-entrypoints/navigation.ts @@ -6,4 +6,4 @@ import '../components/nav-drawer.js'; import '../components/top-app-bar.js'; -import '@material/web/list/list-item-link.js'; +import '@material/web/list/list-item.js'; diff --git a/catalog/src/ssr.ts b/catalog/src/ssr.ts index 61f7b9580..f2fadfacd 100644 --- a/catalog/src/ssr.ts +++ b/catalog/src/ssr.ts @@ -14,7 +14,7 @@ import './components/theme-changer.js'; import '@material/web/menu/menu.js'; import '@material/web/checkbox/checkbox.js'; import '@material/web/list/list.js'; -import '@material/web/list/list-item-link.js'; +import '@material/web/list/list-item.js'; import '@material/web/progress/circular-progress.js'; import '@material/web/tabs/tabs.js'; import '@material/web/tabs/tab.js'; diff --git a/common.ts b/common.ts index 697d87b19..72c903bc1 100644 --- a/common.ts +++ b/common.ts @@ -26,10 +26,8 @@ import './icon/icon.js'; import './iconbutton/icon-button.js'; import './list/list.js'; import './list/list-item.js'; -import './list/list-item-link.js'; import './menu/menu.js'; import './menu/menu-item.js'; -import './menu/menu-item-link.js'; import './menu/sub-menu-item.js'; import './progress/circular-progress.js'; import './progress/linear-progress.js'; @@ -55,10 +53,8 @@ export * from './icon/icon.js'; export * from './iconbutton/icon-button.js'; export * from './list/list.js'; export * from './list/list-item.js'; -export * from './list/list-item-link.js'; export * from './menu/menu.js'; export * from './menu/menu-item.js'; -export * from './menu/menu-item-link.js'; export * from './menu/sub-menu-item.js'; export * from './progress/circular-progress.js'; export * from './progress/linear-progress.js'; diff --git a/list/demo/demo.ts b/list/demo/demo.ts index e33645293..2b34cc64c 100644 --- a/list/demo/demo.ts +++ b/list/demo/demo.ts @@ -48,17 +48,14 @@ const collection = new Knob('supportingText', {ui: textInput(), defaultValue: ''}), new Knob('trailingSupportingText', {ui: textInput(), defaultValue: ''}), new Knob('itemTabIndex', {ui: numberInput(), defaultValue: 0}), + new Knob('href', {ui: textInput(), defaultValue: 'https://google.com'}), + new Knob('target', {ui: textInput(), defaultValue: '_blank'}), + new Knob('link end icon', {ui: textInput(), defaultValue: 'open_in_new'}), new Knob('data-variant=icon', {ui: title()}), new Knob('start icon', {ui: textInput(), defaultValue: 'account_circle'}), new Knob('end icon', {ui: textInput(), defaultValue: 'check_circle'}), - - new Knob('data-variant=link', {ui: title()}), - new Knob('href', {ui: textInput(), defaultValue: 'https://google.com'}), - new Knob('target', {ui: textInput(), defaultValue: '_blank'}), - new Knob('link end icon', {ui: textInput(), defaultValue: 'open_in_new'}), - new Knob('data-variant=avatar', {ui: title()}), new Knob('avatar img', {ui: textInput(), defaultValue: AVATAR_URL}), new Knob('avatar label', {ui: textInput(), defaultValue: 'EM'}), diff --git a/list/demo/stories.ts b/list/demo/stories.ts index ae260c39e..70a69173e 100644 --- a/list/demo/stories.ts +++ b/list/demo/stories.ts @@ -4,7 +4,6 @@ * SPDX-License-Identifier: Apache-2.0 */ -import '@material/web/list/list-item-link.js'; import '@material/web/list/list-item.js'; import '@material/web/divider/divider.js'; import '@material/web/list/list.js'; @@ -110,7 +109,7 @@ const standard: MaterialStoryInit = { - = { .noninteractive=${noninteractive} .itemTabIndex=${itemTabIndex} .href=${href} - .target=${target} + .target=${target as '' | '_blank' | '_parent' | '_self' | '_top'} .active=${active}> ${ knobs['link end icon']} - + diff --git a/list/internal/listitem/list-item.ts b/list/internal/listitem/list-item.ts index e2266e34a..1a5b6a4d6 100644 --- a/list/internal/listitem/list-item.ts +++ b/list/internal/listitem/list-item.ts @@ -10,6 +10,7 @@ import '../../../focus/md-focus-ring.js'; import {html, LitElement, nothing, PropertyValues, TemplateResult} from 'lit'; import {property, query} from 'lit/decorators.js'; import {classMap} from 'lit/directives/class-map.js'; +import {html as staticHtml, literal} from 'lit/static-html.js'; import {ARIAMixinStrict} from '../../../internal/aria/aria.js'; import {requestUpdateOnAriaChange} from '../../../internal/aria/delegate.js'; @@ -82,10 +83,11 @@ export class ListItemEl extends LitElement implements ListItem { @property({type: Boolean, reflect: true}) active = false; /** - * Sets the role of the list item. Set to '' to clear the role. + * Sets the role of the list item. Set to 'nothing' to clear the role. This + * property will be ignored if `href` is set since the underlying element will + * be a native anchor tag. */ - @property() - type: ListItemRole = 'listitem'; + @property() type: ListItemRole = 'listitem'; /** * READONLY. Sets the `md-list-item` attribute on the element. @@ -93,6 +95,17 @@ export class ListItemEl extends LitElement implements ListItem { @property({type: Boolean, attribute: 'md-list-item', reflect: true}) isListItem = true; + /** + * Sets the underlying `HTMLAnchorElement`'s `href` resource attribute. + */ + @property() href = ''; + + /** + * Sets the underlying `HTMLAnchorElement`'s `target` attribute when `href` is + * set. + */ + @property() target: '_blank'|'_parent'|'_self'|'_top'|'' = ''; + @query('.list-item') protected readonly listItemRoot!: HTMLElement|null; /** @@ -134,19 +147,25 @@ export class ListItemEl extends LitElement implements ListItem { * @param content the child content of the list item. */ protected renderListItem(content: unknown) { - return html` -
  • ${content}
    • + >${content} `; } diff --git a/list/internal/listitemlink/list-item-link-only.ts b/list/internal/listitemlink/list-item-link-only.ts deleted file mode 100644 index 2f05f1922..000000000 --- a/list/internal/listitemlink/list-item-link-only.ts +++ /dev/null @@ -1,34 +0,0 @@ -/** - * @license - * Copyright 2023 Google LLC - * SPDX-License-Identifier: Apache-2.0 - */ - -import {nothing} from 'lit'; -import {property} from 'lit/decorators.js'; - -import {ListItemLink} from './list-item-link.js'; - -// tslint:disable-next-line:enforce-comments-on-exported-symbols -export class ListItemLinkOnly extends ListItemLink { - /** - * Removes the hover and click ripples from the item when true. Clicking the - * link will still cause link navigation. - */ - @property({type: Boolean}) noninteractive = false; - - override getRenderClasses() { - return { - ...super.getRenderClasses(), - 'noninteractive': this.noninteractive, - }; - } - - override renderRipple() { - return this.noninteractive ? nothing : super.renderRipple(); - } - - override renderFocusRing() { - return this.noninteractive ? nothing : super.renderFocusRing(); - } -} diff --git a/list/internal/listitemlink/list-item-link.ts b/list/internal/listitemlink/list-item-link.ts deleted file mode 100644 index ebe9d9fab..000000000 --- a/list/internal/listitemlink/list-item-link.ts +++ /dev/null @@ -1,47 +0,0 @@ -/** - * @license - * Copyright 2022 Google LLC - * SPDX-License-Identifier: Apache-2.0 - */ - -import {html, nothing} from 'lit'; -import {property} from 'lit/decorators.js'; -import {classMap} from 'lit/directives/class-map.js'; - -import {ARIAMixinStrict} from '../../../internal/aria/aria.js'; -import {ListItemEl, ListItemRole} from '../listitem/list-item.js'; - -type LinkTarget = '_blank'|'_parent'|'_self'|'_top'; - -// tslint:disable-next-line:enforce-comments-on-exported-symbols -export class ListItemLink extends ListItemEl { - /** - * Sets the underlying `HTMLAnchorElement`'s `href` resource attribute. - */ - @property() href!: string; - - override type: ListItemRole = 'none'; - - /** - * Sets the underlying `HTMLAnchorElement`'s `target` attribute. - */ - @property() target!: string; - protected override renderListItem(content: unknown) { - return html` - ${content} - `; - } -} diff --git a/list/list-item-link.ts b/list/list-item-link.ts deleted file mode 100644 index 6312e169d..000000000 --- a/list/list-item-link.ts +++ /dev/null @@ -1,61 +0,0 @@ -/** - * @license - * Copyright 2022 Google LLC - * SPDX-License-Identifier: Apache-2.0 - */ - -import {customElement} from 'lit/decorators.js'; - -import {styles as forcedColors} from './internal/listitem/forced-colors-styles.css.js'; -import {styles} from './internal/listitem/list-item-styles.css.js'; -import {ListItemLinkOnly as ListItemLink} from './internal/listitemlink/list-item-link-only.js'; - -declare global { - interface HTMLElementTagNameMap { - 'md-list-item-link': MdListItemLink; - } -} - -/** - * @summary - * Lists are continuous, vertical indexes of text or images. Items are placed - * inside the list. This is a linkable variant. - * - * @description - * Lists consist of one or more list items, and can contain actions represented - * by icons and text. List items come in three sizes: one-line, two-line, and - * three-line. - * - * __Takeaways:__ - * - * - Lists should be sorted in logical ways that make content easy to scan, such - * as alphabetical, numerical, chronological, or by user preference. - * - Lists present content in a way that makes it easy to identify a specific - * item in a collection and act on it. - * - Lists should present icons, text, and actions in a consistent format. - * - * Example slottable child variants are: - * - * - `video[data-variant=video]` - * - `img,span[data-variant=avatar]` - * - `img[data-variant=image]` - * - `md-icon[data-variant=icon]` - * - * @example - * ```html - * - * account_circle - * open_in_new - * - * ``` - * - * @final - * @suppress {visibility} - */ -@customElement('md-list-item-link') -export class MdListItemLink extends ListItemLink { - static override styles = [styles, forcedColors]; -} diff --git a/list/list_test.ts b/list/list_test.ts index c109b7560..6e256d5cb 100644 --- a/list/list_test.ts +++ b/list/list_test.ts @@ -7,7 +7,6 @@ // import 'jasmine'; (google3-only) import './list.js'; import './list-item.js'; -import './list-item-link.js'; import {html} from 'lit'; @@ -1101,65 +1100,52 @@ describe('', () => { }); }); -describe('', () => { +describe(' link', () => { const env = new Environment(); describe('.styles', () => { createTokenTests(MdListItem.styles); }); - describe('rendering', () => { - // We repeat these tests because technically list-item-link has its own - // implementation of these methods. Everything else is shared. - it('disabled overrides tabIndex', async () => { - const root = env.render(html``); + it('setting href renders an anchor tag', async () => { + const root = env.render( + html``); - const listItem = root.querySelector('md-list-item-link')!; + const listItem = root.querySelector('md-list-item')!; - await env.waitForStability(); + await env.waitForStability(); - const internalRoot = - listItem.renderRoot.querySelector('#item') as HTMLElement; + const internalRoot = + listItem.renderRoot.querySelector('#item') as HTMLElement; - expect(internalRoot.tabIndex).toBe(-1); + expect(internalRoot.tagName).toBe('A'); + }); + + it('setting type and href does not render a role', async () => { + const root = env.render( + html``); - listItem.itemTabIndex = 2; + const listItem = root.querySelector('md-list-item')!; - await env.waitForStability(); + await env.waitForStability(); - expect(listItem.disabled).toBeFalse(); - expect(internalRoot.tabIndex).toBe(2); + const internalRoot = + listItem.renderRoot.querySelector('#item') as HTMLElement; - listItem.disabled = true; + expect(internalRoot.hasAttribute('role')).toBe(false); + }); + + it('setting target without href renders nothing', async () => { + const root = env.render( + html``); - await env.waitForStability(); + const listItem = root.querySelector('md-list-item')!; - expect(listItem.disabled).toBeTrue(); - expect(internalRoot.tabIndex).toBe(-1); - }); + await env.waitForStability(); - it('ripple and focus ring not rendered on noninteractive', async () => { - const root = env.render(html``); + const internalRoot = + listItem.renderRoot.querySelector('#item') as HTMLElement; - const listItem = root.querySelector('md-list-item-link')!; - - await env.waitForStability(); - - let rippleEl = listItem.renderRoot.querySelector('md-ripple'); - let focusRingEl = listItem.renderRoot.querySelector('md-focus-ring'); - - expect(rippleEl).toBeTruthy(); - expect(focusRingEl).toBeTruthy(); - - listItem.noninteractive = true; - - await env.waitForStability(); - - rippleEl = listItem.renderRoot.querySelector('md-ripple'); - focusRingEl = listItem.renderRoot.querySelector('md-focus-ring'); - - expect(rippleEl).toBeNull(); - expect(focusRingEl).toBeNull(); - }); + expect(internalRoot.hasAttribute('target')).toBe(false); }); }); diff --git a/menu/demo/demo.ts b/menu/demo/demo.ts index c6ed34b0d..b7656df5f 100644 --- a/menu/demo/demo.ts +++ b/menu/demo/demo.ts @@ -108,9 +108,6 @@ const collection = defaultValue: false, ui: boolInput(), }), - - // menu-item-link knobs - new Knob('menu-item-link', {ui: title()}), new Knob('href', { defaultValue: 'https://google.com', ui: textInput(), diff --git a/menu/demo/stories.ts b/menu/demo/stories.ts index 2b74077bd..652e5e96a 100644 --- a/menu/demo/stories.ts +++ b/menu/demo/stories.ts @@ -6,7 +6,6 @@ import '@material/web/menu/menu-item.js'; -import '@material/web/menu/menu-item-link.js'; import '@material/web/menu/sub-menu-item.js'; import '@material/web/menu/menu.js'; import '@material/web/button/filled-button.js'; @@ -41,8 +40,6 @@ export interface StoryKnobs { 'menu-item': void; keepOpen: boolean; disabled: boolean; - - 'menu-item-link': void; href: string; target: string; 'link icon': string; @@ -124,7 +121,7 @@ const standard: MaterialStoryInit = { }; const linkable: MaterialStoryInit = { - name: '', + name: '', styles: sharedStyle, render(knobs) { const showMenu = () => { @@ -262,15 +259,15 @@ function renderItems(names: string[], knobs: StoryKnobs) { function renderLinkableItems(names: string[], knobs: StoryKnobs) { return names.map(name => html` - ${knobs['link icon']} - + `); } diff --git a/menu/internal/menuitemlink/menu-item-link.ts b/menu/internal/menuitemlink/menu-item-link.ts deleted file mode 100644 index 8d084c2cf..000000000 --- a/menu/internal/menuitemlink/menu-item-link.ts +++ /dev/null @@ -1,48 +0,0 @@ -/** - * @license - * Copyright 2023 Google LLC - * SPDX-License-Identifier: Apache-2.0 - */ - -import {property} from 'lit/decorators.js'; - -import {ListItemLink} from '../../../list/internal/listitemlink/list-item-link.js'; -import {CLOSE_REASON, createDefaultCloseMenuEvent, isClosableKey, MenuItem, SELECTION_KEY} from '../shared.js'; - -/** - * @fires close-menu {CloseMenuEvent} - */ -export class MenuItemLink extends ListItemLink implements MenuItem { - /** - * READONLY: self-identifies as a menu item and sets its identifying attribute - */ - @property({type: Boolean, attribute: 'md-menu-item', reflect: true}) - isMenuItem = true; - - /** - * Keeps the menu open if clicked or keyboard selected. - */ - @property({type: Boolean, attribute: 'keep-open'}) keepOpen = false; - - protected keepOpenOnClick = false; - - protected override onClick() { - if (this.keepOpen || this.keepOpenOnClick) return; - - this.dispatchEvent(createDefaultCloseMenuEvent( - this, {kind: CLOSE_REASON.CLICK_SELECTION})); - } - - protected override onKeydown(event: KeyboardEvent) { - if (this.keepOpen) return; - - const keyCode = event.code; - // Do not preventDefault on enter or else it will prevent from opening links - if (!event.defaultPrevented && isClosableKey(keyCode) && - keyCode !== SELECTION_KEY.ENTER) { - event.preventDefault(); - this.dispatchEvent(createDefaultCloseMenuEvent( - this, {kind: CLOSE_REASON.KEYDOWN, key: keyCode})); - } - } -} diff --git a/menu/menu-item-link.ts b/menu/menu-item-link.ts deleted file mode 100644 index 3e2f0e677..000000000 --- a/menu/menu-item-link.ts +++ /dev/null @@ -1,47 +0,0 @@ -/** - * @license - * Copyright 2023 Google LLC - * SPDX-License-Identifier: Apache-2.0 - */ - -import {customElement} from 'lit/decorators.js'; - -import {styles as listItemForcedColorsStyles} from '../list/internal/listitem/forced-colors-styles.css.js'; -import {styles as listItemStyles} from '../list/internal/listitem/list-item-styles.css.js'; - -import {styles as forcedColorsStyles} from './internal/menuitem/forced-colors-styles.css.js'; -import {styles} from './internal/menuitem/menu-item-styles.css.js'; -import {MenuItemLink} from './internal/menuitemlink/menu-item-link.js'; - -export {ListItem} from '../list/internal/listitem/list-item.js'; -export {CloseMenuEvent, MenuItem} from './internal/shared.js'; - - -declare global { - interface HTMLElementTagNameMap { - 'md-menu-item-link': MdMenuItemLink; - } -} - -/** - * @summary Menus display a list of choices on a temporary surface. - * - * @description - * Menu items are the selectable choices within the menu. Menu items must - * implement the `MenuItem` interface and also have the `md-menu-item` - * attribute. Additionally menu items are list items so they must also have the - * `md-list-item` attribute. - * - * Menu items can control a menu by selectively firing the `close-menu` and - * `deselect-items` events. - * - * This is a linkable variant. - * - * @final - * @suppress {visibility} - */ -@customElement('md-menu-item-link') -export class MdMenuItemLink extends MenuItemLink { - static override styles = - [listItemStyles, styles, listItemForcedColorsStyles, forcedColorsStyles]; -}