From ab4e01b2d56cce6be617bb13c13c0b9526bcc1ff Mon Sep 17 00:00:00 2001 From: Tomi Virkki Date: Wed, 2 Oct 2024 16:13:15 +0300 Subject: [PATCH] move the title element inside the shadow root --- packages/dashboard/src/title-controller.js | 76 ------------------- .../dashboard/src/vaadin-dashboard-section.js | 26 +------ .../dashboard/src/vaadin-dashboard-widget.js | 31 +++----- .../dashboard/test/dashboard-layout.test.ts | 3 +- .../dashboard/test/dashboard-section.test.ts | 23 +----- .../dashboard/test/dashboard-widget.test.ts | 53 +++---------- packages/dashboard/test/helpers.ts | 4 + .../lumo/vaadin-dashboard-widget-styles.js | 10 ++- 8 files changed, 40 insertions(+), 186 deletions(-) delete mode 100644 packages/dashboard/src/title-controller.js diff --git a/packages/dashboard/src/title-controller.js b/packages/dashboard/src/title-controller.js deleted file mode 100644 index 7549cc1dd1..0000000000 --- a/packages/dashboard/src/title-controller.js +++ /dev/null @@ -1,76 +0,0 @@ -/** - * @license - * Copyright (c) 2019 - 2024 Vaadin Ltd. - * This program is available under Apache License Version 2.0, available at https://vaadin.com/license/ - */ -import { SlotChildObserveController } from '@vaadin/component-base/src/slot-child-observe-controller.js'; - -const DEFAULT_TITLE_LEVEL = '2'; - -/** - * A controller to manage the widget or section title element. - */ -export class TitleController extends SlotChildObserveController { - constructor(host) { - super(host, 'title', null); - } - - /** - * Set title based on corresponding host property. - * - * @param {string} title - */ - setTitle(title) { - this.title = title; - - const titleLevel = - getComputedStyle(this.host).getPropertyValue('--_vaadin-dashboard-title-level') || DEFAULT_TITLE_LEVEL; - const newTagName = `h${titleLevel}`; - if (this.tagName !== newTagName) { - if (this.defaultNode) { - this.defaultNode.remove(); - delete this.defaultNode; - } - this.tagName = newTagName; - } - - // Restore the default title, if needed. - const titleNode = this.getSlotChild(); - if (!titleNode) { - this.restoreDefaultNode(); - } - - // When default title is used, update it. - if (this.node === this.defaultNode) { - this.updateDefaultNode(this.node); - } - } - - /** - * Override method inherited from `SlotChildObserveController` - * to restore and observe the default title element. - * - * @protected - * @override - */ - restoreDefaultNode() { - this.attachDefaultNode(); - } - - /** - * Override method inherited from `SlotChildObserveController` - * to update the default title element text content. - * - * @param {Node | undefined} node - * @protected - * @override - */ - updateDefaultNode(node) { - if (node) { - node.textContent = this.title; - } - - // Notify the host after update. - super.updateDefaultNode(node); - } -} diff --git a/packages/dashboard/src/vaadin-dashboard-section.js b/packages/dashboard/src/vaadin-dashboard-section.js index 78814dda32..11c1a1cfe8 100644 --- a/packages/dashboard/src/vaadin-dashboard-section.js +++ b/packages/dashboard/src/vaadin-dashboard-section.js @@ -14,7 +14,6 @@ import { defineCustomElement } from '@vaadin/component-base/src/define.js'; import { ElementMixin } from '@vaadin/component-base/src/element-mixin.js'; import { PolylitMixin } from '@vaadin/component-base/src/polylit-mixin.js'; import { css, ThemableMixin } from '@vaadin/vaadin-themable-mixin/vaadin-themable-mixin.js'; -import { TitleController } from './title-controller.js'; import { DashboardItemMixin } from './vaadin-dashboard-item-mixin.js'; import { getDefaultI18n } from './vaadin-dashboard-item-mixin.js'; import { hasWidgetWrappers } from './vaadin-dashboard-styles.js'; @@ -181,7 +180,6 @@ class DashboardSection extends DashboardItemMixin( sectionTitle: { type: String, value: '', - observer: '__onSectionTitleChanged', }, }; } @@ -194,7 +192,7 @@ class DashboardSection extends DashboardItemMixin(
${this.__renderDragHandle()} - +

${this.sectionTitle}

${this.__renderRemoveButton()}
@@ -203,30 +201,10 @@ class DashboardSection extends DashboardItemMixin( `; } - constructor() { - super(); - this.__titleController = new TitleController(this); - this.__titleController.addEventListener('slot-content-changed', (event) => { - const { node } = event.target; - if (node) { - this.setAttribute('aria-labelledby', node.id); - } - }); - } - /** @protected */ ready() { super.ready(); - this.addController(this.__titleController); - - if (!this.hasAttribute('role')) { - this.setAttribute('role', 'section'); - } - } - - /** @private */ - __onSectionTitleChanged(sectionTitle) { - this.__titleController.setTitle(sectionTitle); + this.role ??= 'section'; } } diff --git a/packages/dashboard/src/vaadin-dashboard-widget.js b/packages/dashboard/src/vaadin-dashboard-widget.js index 8036b3afeb..6949443756 100644 --- a/packages/dashboard/src/vaadin-dashboard-widget.js +++ b/packages/dashboard/src/vaadin-dashboard-widget.js @@ -15,7 +15,6 @@ import { defineCustomElement } from '@vaadin/component-base/src/define.js'; import { ElementMixin } from '@vaadin/component-base/src/element-mixin.js'; import { PolylitMixin } from '@vaadin/component-base/src/polylit-mixin.js'; import { css, ThemableMixin } from '@vaadin/vaadin-themable-mixin/vaadin-themable-mixin.js'; -import { TitleController } from './title-controller.js'; import { SYNCHRONIZED_ATTRIBUTES, WRAPPER_LOCAL_NAME } from './vaadin-dashboard-helpers.js'; import { DashboardItemMixin } from './vaadin-dashboard-item-mixin.js'; import { getDefaultI18n } from './vaadin-dashboard-item-mixin.js'; @@ -200,6 +199,12 @@ class DashboardWidget extends DashboardItemMixin( value: '', observer: '__onWidgetTitleChanged', }, + + /* @private */ + __nestedHeadingLevel: { + type: Boolean, + value: false, + }, }; } @@ -211,7 +216,9 @@ class DashboardWidget extends DashboardItemMixin(
${this.__renderDragHandle()} - + ${this.__nestedHeadingLevel + ? html`

${this.widgetTitle}

` + : html`

${this.widgetTitle}

`} ${this.__renderRemoveButton()}
@@ -225,17 +232,6 @@ class DashboardWidget extends DashboardItemMixin( `; } - constructor() { - super(); - this.__titleController = new TitleController(this); - this.__titleController.addEventListener('slot-content-changed', (event) => { - const { node } = event.target; - if (node) { - this.setAttribute('aria-labelledby', node.id); - } - }); - } - /** @protected */ connectedCallback() { super.connectedCallback(); @@ -259,11 +255,7 @@ class DashboardWidget extends DashboardItemMixin( /** @protected */ ready() { super.ready(); - this.addController(this.__titleController); - - if (!this.hasAttribute('role')) { - this.setAttribute('role', 'article'); - } + this.role ??= 'article'; } /** @private */ @@ -273,7 +265,8 @@ class DashboardWidget extends DashboardItemMixin( /** @private */ __updateTitle() { - this.__titleController.setTitle(this.widgetTitle); + const titleLevel = getComputedStyle(this).getPropertyValue('--_vaadin-dashboard-title-level'); + this.__nestedHeadingLevel = titleLevel === '3'; } } diff --git a/packages/dashboard/test/dashboard-layout.test.ts b/packages/dashboard/test/dashboard-layout.test.ts index 4422b5908e..d6f632f215 100644 --- a/packages/dashboard/test/dashboard-layout.test.ts +++ b/packages/dashboard/test/dashboard-layout.test.ts @@ -10,6 +10,7 @@ import { getColumnWidths, getRowHeights, getScrollingContainer, + getTitleElement, onceResized, setColspan, setMaximumColumnCount, @@ -511,7 +512,7 @@ describe('dashboard layout', () => { }); it('should not use minimum row height for section header row', async () => { - const title = section.querySelector('[slot="title"]')!; + const title = getTitleElement(section); title.style.height = '100%'; const titleHeight = title.offsetHeight; diff --git a/packages/dashboard/test/dashboard-section.test.ts b/packages/dashboard/test/dashboard-section.test.ts index 851dc36fe2..731b63d00a 100644 --- a/packages/dashboard/test/dashboard-section.test.ts +++ b/packages/dashboard/test/dashboard-section.test.ts @@ -8,6 +8,7 @@ import { getMoveBackwardButton, getMoveForwardButton, getRemoveButton, + getTitleElement, } from './helpers.js'; describe('dashboard section', () => { @@ -40,28 +41,10 @@ describe('dashboard section', () => { expect(section.getAttribute('role')).to.eql('region'); }); - it('should add title id to aria-labelledby attribute when using property', async () => { - section.sectionTitle = 'Custom title'; - await nextFrame(); - const title = section.querySelector('[slot="title"]'); - expect(section.getAttribute('aria-labelledby')).equal(title?.id); - }); - - it('should add title id to aria-labelledby attribute when using slot', async () => { - const title = document.createElement('div'); - title.id = 'custom-title'; - title.slot = 'title'; - title.textContent = 'Custom title'; - section.appendChild(title); - - await nextFrame(); - expect(section.getAttribute('aria-labelledby')).equal(title?.id); - }); - it('should have text content for the title', async () => { section.sectionTitle = 'Custom title'; await nextFrame(); - const title = section.querySelector('[slot="title"]'); + const title = getTitleElement(section); expect(title?.textContent).equal('Custom title'); }); }); @@ -102,7 +85,7 @@ describe('dashboard section', () => { section.sectionTitle = null; await nextFrame(); - const title = section.querySelector('[slot="title"]'); + const title = getTitleElement(section); expect(title?.textContent).to.eql(''); }); }); diff --git a/packages/dashboard/test/dashboard-widget.test.ts b/packages/dashboard/test/dashboard-widget.test.ts index d27a7f5504..d1c1a08fc8 100644 --- a/packages/dashboard/test/dashboard-widget.test.ts +++ b/packages/dashboard/test/dashboard-widget.test.ts @@ -15,6 +15,7 @@ import { getResizeHandle, getResizeShrinkHeightButton, getResizeShrinkWidthButton, + getTitleElement, } from './helpers.js'; describe('dashboard widget', () => { @@ -42,28 +43,10 @@ describe('dashboard widget', () => { expect(widget.getAttribute('role')).to.eql('region'); }); - it('should add title id to aria-labelledby attribute when using property', async () => { - widget.widgetTitle = 'Custom title'; - await nextFrame(); - const title = widget.querySelector('[slot="title"]'); - expect(widget.getAttribute('aria-labelledby')).equal(title?.id); - }); - - it('should add title id to aria-labelledby attribute when using slot', async () => { - const title = document.createElement('div'); - title.id = 'custom-title'; - title.slot = 'title'; - title.textContent = 'Custom title'; - widget.appendChild(title); - - await nextFrame(); - expect(widget.getAttribute('aria-labelledby')).equal(title?.id); - }); - it('should have text content for the title', async () => { widget.widgetTitle = 'Custom title'; await nextFrame(); - const title = widget.querySelector('[slot="title"]'); + const title = getTitleElement(widget); expect(title?.textContent).equal('Custom title'); }); }); @@ -104,7 +87,7 @@ describe('dashboard widget', () => { widget.widgetTitle = null; await nextFrame(); - const title = widget.querySelector('[slot="title"]'); + const title = getTitleElement(widget); expect(title?.textContent).to.eql(''); }); }); @@ -201,7 +184,7 @@ describe('widget title level', () => { const widget = fixtureSync(``); await nextFrame(); - const title = widget.querySelector('[slot="title"]'); + const title = getTitleElement(widget as DashboardWidget); expect(title?.localName).to.equal('h2'); }); @@ -209,7 +192,7 @@ describe('widget title level', () => { const section = fixtureSync(``); await nextFrame(); - const title = section.querySelector('[slot="title"]'); + const title = getTitleElement(section as DashboardSection); expect(title?.localName).to.equal('h2'); }); @@ -221,7 +204,7 @@ describe('widget title level', () => { `).querySelector('vaadin-dashboard-widget')!; await nextFrame(); - const title = widget.querySelector('[slot="title"]'); + const title = getTitleElement(widget); expect(title?.localName).to.equal('h3'); }); @@ -239,7 +222,7 @@ describe('widget title level', () => { wrapper.appendChild(widget); await nextFrame(); - const title = widget.querySelector('[slot="title"]'); + const title = getTitleElement(widget); expect(title?.localName).to.equal('h2'); }); @@ -256,7 +239,7 @@ describe('widget title level', () => { section.appendChild(widget); await nextFrame(); - const title = widget.querySelector('[slot="title"]'); + const title = getTitleElement(widget); expect(title?.localName).to.equal('h3'); }); @@ -272,7 +255,7 @@ describe('widget title level', () => { customElements.define('my-custom-section', MyCustomSection); await nextFrame(); - const title = widget.querySelector('[slot="title"]'); + const title = getTitleElement(widget); expect(title?.localName).to.equal('h3'); }); @@ -288,7 +271,7 @@ describe('widget title level', () => { customElements.define('my-custom-widget', MyCustomWidget); await nextFrame(); - const title = widget.querySelector('[slot="title"]'); + const title = getTitleElement(widget as DashboardWidget); expect(title?.localName).to.equal('h3'); }); @@ -308,21 +291,7 @@ describe('widget title level', () => { section.appendChild(wrapper); await nextFrame(); - const title = widget.querySelector('[slot="title"]'); + const title = getTitleElement(widget); expect(title?.localName).to.equal('h3'); }); - - it('should not replace an explicitly defined widget title element', async () => { - const widget = fixtureSync(` - - -

foo

- - - `).querySelector('vaadin-dashboard-widget')!; - await nextFrame(); - - const title = widget.querySelector('[slot="title"]'); - expect(title?.localName).to.equal('h2'); - }); }); diff --git a/packages/dashboard/test/helpers.ts b/packages/dashboard/test/helpers.ts index 4d0998db2f..0b4374b69d 100644 --- a/packages/dashboard/test/helpers.ts +++ b/packages/dashboard/test/helpers.ts @@ -70,6 +70,10 @@ export function getRowHeights(dashboard: HTMLElement): number[] { return dashboardRowHeights; } +export function getTitleElement(item: DashboardWidget | DashboardSection): HTMLElement { + return item.shadowRoot!.querySelector('#title')!; +} + /** * Returns the element at the center of the cell at the given row and column index. */ diff --git a/packages/dashboard/theme/lumo/vaadin-dashboard-widget-styles.js b/packages/dashboard/theme/lumo/vaadin-dashboard-widget-styles.js index 1379f12f5d..62d9b2135e 100644 --- a/packages/dashboard/theme/lumo/vaadin-dashboard-widget-styles.js +++ b/packages/dashboard/theme/lumo/vaadin-dashboard-widget-styles.js @@ -87,12 +87,14 @@ const dashboardWidgetAndSection = css` --icon: var(--lumo-icons-cross); } - /* Title slot styling */ - slot[name='title']::slotted(*) { + /* Title styling */ + h2, + h3 { flex: 1; - font-size: var(--lumo-font-size-m) !important; + font-size: var(--lumo-font-size-m); font-weight: 500; - color: var(--lumo-header-text-color) !important; + color: var(--lumo-header-text-color); + margin: 0; } /* Content styling */