From 0467c4845db7c31deddf7383beb199157f4c5034 Mon Sep 17 00:00:00 2001 From: Elizabeth Mitchell Date: Tue, 29 Aug 2023 11:33:10 -0700 Subject: [PATCH] fix(tabs): incorrect layout and primary indicator width PiperOrigin-RevId: 561091366 --- tabs/internal/_secondary-tab.scss | 9 ++---- tabs/internal/_tab.scss | 15 ++++----- tabs/internal/secondary-tab.ts | 4 ++- tabs/internal/tab.ts | 53 +++++++++++++++++++++++++------ tokens/_md-comp-primary-tab.scss | 4 +-- 5 files changed, 57 insertions(+), 28 deletions(-) diff --git a/tabs/internal/_secondary-tab.scss b/tabs/internal/_secondary-tab.scss index 22bcedd81..97cfa94c0 100644 --- a/tabs/internal/_secondary-tab.scss +++ b/tabs/internal/_secondary-tab.scss @@ -57,13 +57,8 @@ --md-secondary-tab-container-shape-end-start, var(--_container-shape) ); - } - .content { - width: 100%; - } - - .indicator { - min-width: 100%; + // TODO(b/293503309): remove once secondary tabs only use inline icons + --_with-icon-and-label-text-container-height: 64px; } } diff --git a/tabs/internal/_tab.scss b/tabs/internal/_tab.scss index 8ea5a305d..2a339ed50 100644 --- a/tabs/internal/_tab.scss +++ b/tabs/internal/_tab.scss @@ -57,7 +57,7 @@ text-decoration: none; width: 100%; position: relative; - padding: 0; + padding: 0 16px; margin: 0; z-index: 0; // Ensure this is a stacking context so the indicator displays font: var(--_label-text-type); @@ -92,18 +92,17 @@ flex-direction: column; align-items: center; justify-content: center; + height: var(--_container-height); + gap: 2px; + } - $_content-padding: 8px; - // tabs are naturally sized up to their max height. - max-height: calc(var(--_container-height) + 2 * $_content-padding); - // min-height of touch target - min-height: 48px; - padding: $_content-padding calc(2 * $_content-padding); - gap: 4px; + .content.has-icon.has-label:not(.inline-icon) { + height: var(--_with-icon-and-label-text-container-height); } .content.inline-icon { flex-direction: row; + gap: 8px; } .indicator { diff --git a/tabs/internal/secondary-tab.ts b/tabs/internal/secondary-tab.ts index 9eb30227e..086101f34 100644 --- a/tabs/internal/secondary-tab.ts +++ b/tabs/internal/secondary-tab.ts @@ -9,4 +9,6 @@ import {Tab} from './tab.js'; /** * A secondary tab component. */ -export class SecondaryTab extends Tab {} +export class SecondaryTab extends Tab { + protected override fullWidthIndicator = true; +} diff --git a/tabs/internal/tab.ts b/tabs/internal/tab.ts index 67e8b59f8..41b44a4f0 100644 --- a/tabs/internal/tab.ts +++ b/tabs/internal/tab.ts @@ -9,7 +9,7 @@ import '../../focus/md-focus-ring.js'; import '../../ripple/ripple.js'; import {html, isServer, LitElement, nothing, PropertyValues} from 'lit'; -import {property, query} from 'lit/decorators.js'; +import {property, query, queryAssignedElements, queryAssignedNodes, state} from 'lit/decorators.js'; import {classMap} from 'lit/directives/class-map.js'; import {ARIAMixinStrict} from '../../internal/aria/aria.js'; @@ -56,13 +56,26 @@ export class Tab extends LitElement { */ @property({type: Boolean, attribute: 'inline-icon'}) inlineIcon = false; + /** + * In SSR, set this to true when an icon is present. + */ + @property({type: Boolean, attribute: 'has-icon'}) hasIcon = false; + + /** + * In SSR, set this to true when there is no label and only an icon. + */ + @property({type: Boolean, attribute: 'icon-only'}) iconOnly = false; + @query('.button') private readonly button!: HTMLElement|null; // note, this is public so it can participate in selection animation. - /** - * Selection indicator element. - */ + /** @private */ @query('.indicator') readonly indicator!: HTMLElement; + @state() protected fullWidthIndicator = false; + @queryAssignedNodes({flatten: true}) + private readonly assignedDefaultNodes!: Node[]; + @queryAssignedElements({slot: 'icon', flatten: true}) + private readonly assignedIcons!: HTMLElement[]; constructor() { super(); @@ -82,7 +95,11 @@ export class Tab extends LitElement { protected override render() { const contentClasses = { 'inline-icon': this.inlineIcon, + 'has-icon': this.hasIcon, + 'has-label': !this.iconOnly, }; + + const indicator = html`
`; // Needed for closure conformance const {ariaLabel} = this as ARIAMixinStrict; return html` @@ -97,12 +114,11 @@ export class Tab extends LitElement {
- - - - -
+ + + ${this.fullWidthIndicator ? nothing : indicator}
+ ${this.fullWidthIndicator ? indicator : nothing} `; } @@ -161,6 +177,25 @@ export class Tab extends LitElement { // that can hide the animation. return [from, {'transform': 'none'}]; } + + private handleSlotChange() { + this.iconOnly = false; + // Check if there's any label text or elements. If not, then there is only + // an icon. + for (const node of this.assignedDefaultNodes) { + const hasTextContent = node.nodeType === Node.TEXT_NODE && + !!(node as Text).wholeText.match(/\S/); + if (node.nodeType === Node.ELEMENT_NODE || hasTextContent) { + return; + } + } + + this.iconOnly = true; + } + + private handleIconSlotChange() { + this.hasIcon = this.assignedIcons.length > 0; + } } function shouldReduceMotion() { diff --git a/tokens/_md-comp-primary-tab.scss b/tokens/_md-comp-primary-tab.scss index ba4085820..6594e0d92 100644 --- a/tokens/_md-comp-primary-tab.scss +++ b/tokens/_md-comp-primary-tab.scss @@ -59,13 +59,11 @@ $supported-tokens: ( 'pressed-label-text-color', 'pressed-state-layer-color', 'pressed-state-layer-opacity', + 'with-icon-and-label-text-container-height', // go/keep-sorted end ); $unsupported-tokens: ( - // include an icon and the size will adjust; - // height is 48 and it's 64 with icon - 'with-icon-and-label-text-container-height', 'with-label-text-label-text-font', 'with-label-text-label-text-line-height', 'with-label-text-label-text-size',