From 4b1c09b3f0b4f0306513ece006b1fefec7fc918c Mon Sep 17 00:00:00 2001 From: Serhii Kulykov Date: Wed, 12 Oct 2022 10:02:35 +0300 Subject: [PATCH] refactor!: move vaadin-message elements to light DOM (#4710) --- .../message-list/src/vaadin-message-list.d.ts | 8 +- .../message-list/src/vaadin-message-list.js | 97 ++++-- .../__snapshots__/message-list.test.snap.js | 36 +-- .../test/dom/message-list.test.js | 6 +- .../message-list/test/message-list.test.js | 279 ++++++++++-------- .../test/visual/lumo/message-list.test.js | 2 +- .../test/visual/material/message-list.test.js | 2 +- 7 files changed, 251 insertions(+), 179 deletions(-) diff --git a/packages/message-list/src/vaadin-message-list.d.ts b/packages/message-list/src/vaadin-message-list.d.ts index ecf735f2522..7594873142b 100644 --- a/packages/message-list/src/vaadin-message-list.d.ts +++ b/packages/message-list/src/vaadin-message-list.d.ts @@ -21,12 +21,15 @@ export interface MessageListItem { * `` is a Web Component for showing an ordered list of messages. The messages are rendered as * * ### Example + * * To create a new message list, add the component to the page: + * * ```html * * ``` * - * Provide the messages to the message list with the `items` property. + * Provide the messages to the message list with the [`items`](#/elements/vaadin-message-list#property-items) property. + * * ```js * document.querySelector('vaadin-message-list').items = [ * { text: 'Hello list', time: 'yesterday', userName: 'Matt Mambo', userAbbr: 'MM', userColorIndex: 1 }, @@ -42,6 +45,9 @@ export interface MessageListItem { * ----------|---------------- * `list` | The container wrapping messages. * + * See the [``](#/elements/vaadin-message) documentation for the available + * state attributes and stylable shadow parts of message elements. + * * See [Styling Components](https://vaadin.com/docs/latest/styling/custom-theme/styling-components) documentation. */ declare class MessageList extends KeyboardDirectionMixin(ThemableMixin(ElementMixin(HTMLElement))) { diff --git a/packages/message-list/src/vaadin-message-list.js b/packages/message-list/src/vaadin-message-list.js index a4e6d4df538..0bbbcb99c90 100644 --- a/packages/message-list/src/vaadin-message-list.js +++ b/packages/message-list/src/vaadin-message-list.js @@ -3,9 +3,7 @@ * Copyright (c) 2021 - 2022 Vaadin Ltd. * This program is available under Apache License Version 2.0, available at https://vaadin.com/license/ */ -import '@polymer/polymer/lib/elements/dom-repeat.js'; import { html, PolymerElement } from '@polymer/polymer/polymer-element.js'; -import { microTask } from '@vaadin/component-base/src/async.js'; import { ElementMixin } from '@vaadin/component-base/src/element-mixin.js'; import { KeyboardDirectionMixin } from '@vaadin/component-base/src/keyboard-direction-mixin.js'; import { ThemableMixin } from '@vaadin/vaadin-themable-mixin/vaadin-themable-mixin.js'; @@ -15,12 +13,15 @@ import { Message } from './vaadin-message.js'; * `` is a Web Component for showing an ordered list of messages. The messages are rendered as * * ### Example + * * To create a new message list, add the component to the page: + * * ```html * * ``` * - * Provide the messages to the message list with the `items` property. + * Provide the messages to the message list with the [`items`](#/elements/vaadin-message-list#property-items) property. + * * ```js * document.querySelector('vaadin-message-list').items = [ * { text: 'Hello list', time: 'yesterday', userName: 'Matt Mambo', userAbbr: 'MM', userColorIndex: 1 }, @@ -36,6 +37,9 @@ import { Message } from './vaadin-message.js'; * ----------|---------------- * `list` | The container wrapping messages. * + * See the [``](#/elements/vaadin-message) documentation for the available + * state attributes and stylable shadow parts of message elements. + * * See [Styling Components](https://vaadin.com/docs/latest/styling/custom-theme/styling-components) documentation. * * @extends HTMLElement @@ -86,19 +90,7 @@ class MessageList extends KeyboardDirectionMixin(ElementMixin(ThemableMixin(Poly }
- +
`; } @@ -126,24 +118,81 @@ class MessageList extends KeyboardDirectionMixin(ElementMixin(ThemableMixin(Poly /** @protected */ get _messages() { - return Array.from(this.shadowRoot.querySelectorAll('vaadin-message')); + return [...this.querySelectorAll('vaadin-message')]; } /** @private */ _itemsChanged(newVal, oldVal) { - const focusedIndex = this._getIndexOfFocusableElement(); - if (newVal && newVal.length) { - const moreItems = !oldVal || newVal.length > oldVal.length; + const items = newVal || []; + const oldItems = oldVal || []; + + if (items.length || oldItems.length) { + const focusedIndex = this._getIndexOfFocusableElement(); const closeToBottom = this.scrollHeight < this.clientHeight + this.scrollTop + 50; - microTask.run(() => { - this._setTabIndexesByIndex(focusedIndex); - if (moreItems && closeToBottom) { + + const removed = oldItems.filter((item) => !items.includes(item)); + const added = [...items]; + + this._messages.forEach((message) => { + const item = message._item; + if (removed.includes(item)) { + message.remove(); + } else if (added.includes(item)) { + added.splice(added.indexOf(item), 1); + } + }); + + this.__addMessages(added, items); + + this._setTabIndexesByIndex(focusedIndex); + + requestAnimationFrame(() => { + if (items.length > oldItems.length && closeToBottom) { this._scrollToLastMessage(); } }); } } + /** @private */ + __addMessages(itemsToAdd, allItems) { + itemsToAdd.forEach((item) => { + const message = this.__createMessage(item); + const nextItem = allItems[allItems.indexOf(item) + 1]; + const nextMessage = this._messages.find((msg) => msg._item === nextItem); + if (nextMessage) { + this.insertBefore(message, nextMessage); + } else { + this.appendChild(message); + } + }); + } + + /** @private */ + __createMessage(item) { + const message = document.createElement('vaadin-message'); + message.setAttribute('role', 'listitem'); + + message.textContent = item.text; + message.time = item.time; + message.userName = item.userName; + message.userAbbr = item.userAbbr; + message.userImg = item.userImg; + message.userColorIndex = item.userColorIndex; + + message._item = item; + + if (item.theme) { + message.setAttribute('theme', item.theme); + } + + message.addEventListener('focusin', (e) => { + this._onMessageFocusIn(e); + }); + + return message; + } + /** @private */ _scrollToLastMessage() { if (this.items.length > 0) { @@ -152,7 +201,7 @@ class MessageList extends KeyboardDirectionMixin(ElementMixin(ThemableMixin(Poly } /** @private */ - _handleFocusEvent(e) { + _onMessageFocusIn(e) { const target = e.composedPath().find((node) => node instanceof Message); this._setTabIndexesByMessage(target); } diff --git a/packages/message-list/test/dom/__snapshots__/message-list.test.snap.js b/packages/message-list/test/dom/__snapshots__/message-list.test.snap.js index 8c9454b9396..aca1457deb4 100644 --- a/packages/message-list/test/dom/__snapshots__/message-list.test.snap.js +++ b/packages/message-list/test/dom/__snapshots__/message-list.test.snap.js @@ -2,22 +2,18 @@ export const snapshots = {}; snapshots["vaadin-message-list default"] = -`
- - - -
+ `; /* end snapshot vaadin-message-list default */ snapshots["vaadin-message-list items"] = -`
- - - -
+ `; /* end snapshot vaadin-message-list items */ snapshots["vaadin-message-list theme"] = -`
- - - -
+ `; /* end snapshot vaadin-message-list theme */ diff --git a/packages/message-list/test/dom/message-list.test.js b/packages/message-list/test/dom/message-list.test.js index 7f88ab43118..995f38f7970 100644 --- a/packages/message-list/test/dom/message-list.test.js +++ b/packages/message-list/test/dom/message-list.test.js @@ -10,7 +10,7 @@ describe('vaadin-message-list', () => { }); it('default', async () => { - await expect(list).shadowDom.to.equalSnapshot(); + await expect(list).dom.to.equalSnapshot(); }); it('items', async () => { @@ -19,12 +19,12 @@ describe('vaadin-message-list', () => { { text: 'Good morning!', userName: 'Lina Roy' }, ]; await nextFrame(); - await expect(list).shadowDom.to.equalSnapshot(); + await expect(list).dom.to.equalSnapshot(); }); it('theme', async () => { list.items = [{ text: 'Partial service outage.', userName: 'Admin', theme: 'danger' }]; await nextFrame(); - await expect(list).shadowDom.to.equalSnapshot(); + await expect(list).dom.to.equalSnapshot(); }); }); diff --git a/packages/message-list/test/message-list.test.js b/packages/message-list/test/message-list.test.js index e13dd29ca10..49904657b0c 100644 --- a/packages/message-list/test/message-list.test.js +++ b/packages/message-list/test/message-list.test.js @@ -89,8 +89,8 @@ describe('message-list', () => { await nextRender(messageList); }); - it('message list should have two messages', () => { - const items = messageList.shadowRoot.querySelectorAll('vaadin-message'); + it('should render vaadin-message element for each item', () => { + const items = messageList.querySelectorAll('vaadin-message'); expect(items.length).to.equal(4); }); @@ -101,7 +101,7 @@ describe('message-list', () => { }); it('message properties should be correctly set', () => { - const firstMessage = messageList.shadowRoot.querySelector('vaadin-message'); + const firstMessage = messageList.querySelector('vaadin-message'); expect(firstMessage.time).to.be.equal(messages[0].time); expect(firstMessage.userName).to.be.equal(messages[0].userName); expect(firstMessage.userAbbr).to.be.equal(messages[0].userAbbr); @@ -111,142 +111,171 @@ describe('message-list', () => { expect(firstMessage.theme).to.be.equal(messages[0].theme); }); - it('message list should scroll when height is less than content', () => { - messageList.style.height = '100px'; - expect(messageList.scrollTop).to.be.equal(0); - messageList.scrollBy(0, 1000); - expect(messageList.scrollTop).to.be.at.least(1); + describe('adding / removing', () => { + it('should remove vaadin-message elements for removed items', async () => { + messageList.items = [messages[1], messages[2]]; + await nextRender(); + const items = messageList.querySelectorAll('vaadin-message'); + expect(items.length).to.equal(2); + }); + + it('should append vaadin-message elements for items added at the end', async () => { + messageList.items = [messages[1], messages[2], { userAbbr: 'SK', text: 'Hi' }]; + await nextRender(); + const items = messageList.querySelectorAll('vaadin-message'); + expect(items.length).to.equal(3); + expect(items[2].textContent).to.equal('Hi'); + }); + + it('should insert vaadin-message elements for items added in the middle', async () => { + messageList.items = [messages[1], { userAbbr: 'SK', text: 'Hi' }, messages[2]]; + await nextRender(); + const items = messageList.querySelectorAll('vaadin-message'); + expect(items.length).to.equal(3); + expect(items[1].textContent).to.equal('Hi'); + }); }); - it('message list should scroll to bottom on new messages', async () => { - messageList.style.height = '100px'; - messageList.scrollBy(0, 1000); - const scrollTopBeforeMessage = messageList.scrollTop; - messageList.items = [ - ...messageList.items, - { - text: 'A new message arrives!', - time: '2:35 PM', - user: { - name: 'Steve Mops', - abbr: 'SM', - colorIndex: 2, + describe('scroll', () => { + it('should scroll when height is less than content', () => { + messageList.style.height = '100px'; + expect(messageList.scrollTop).to.be.equal(0); + messageList.scrollBy(0, 1000); + expect(messageList.scrollTop).to.be.at.least(1); + }); + + it('should scroll to bottom on adding new messages', async () => { + messageList.style.height = '100px'; + messageList.scrollBy(0, 1000); + const scrollTopBeforeMessage = messageList.scrollTop; + messageList.items = [ + ...messageList.items, + { + text: 'A new message arrives!', + time: '2:35 PM', + user: { + name: 'Steve Mops', + abbr: 'SM', + colorIndex: 2, + }, }, - }, - ]; - await nextRender(messageList); - expect(messageList.scrollTop).to.be.at.least(scrollTopBeforeMessage + 1); - }); + ]; + await nextRender(messageList); + expect(messageList.scrollTop).to.be.at.least(scrollTopBeforeMessage + 1); + }); - it('message list should not scroll if not at the bottom', async () => { - messageList.style.height = '100px'; - messageList.items = [ - ...messageList.items, - { - text: 'A new message arrives!', - time: '2:35 PM', - user: { - name: 'Steve Mops', - abbr: 'SM', - colorIndex: 2, + it('should not scroll if not at the bottom', async () => { + messageList.style.height = '100px'; + messageList.items = [ + ...messageList.items, + { + text: 'A new message arrives!', + time: '2:35 PM', + user: { + name: 'Steve Mops', + abbr: 'SM', + colorIndex: 2, + }, }, - }, - ]; - await nextRender(messageList); - expect(messageList.scrollTop).to.be.equal(0); + ]; + await nextRender(messageList); + expect(messageList.scrollTop).to.be.equal(0); + }); }); - it('message list should set tab index on first item if new item list is shorter, and it does not have a item index corresponding to the previous item with tab index 0', async () => { - const messages = messageList.shadowRoot.querySelectorAll('vaadin-message'); - const thirdMessage = messages[2]; - - // Click on third item to give it tabindex=0 - thirdMessage.dispatchEvent(new CustomEvent('mousedown', { composed: true, bubbles: true })); - thirdMessage.dispatchEvent(new CustomEvent('focus', { composed: true, bubbles: true })); - thirdMessage.dispatchEvent(new CustomEvent('mouseup', { composed: true, bubbles: true })); - - // Set message list to shorter than three items, so that tabIndex=0 can't be maintained on third item - messageList.items = [ - { - text: 'This is a new list', - time: '2:35 PM', - user: { - name: 'Steve Mops', - abbr: 'SM', - colorIndex: 2, + describe('tabindex', () => { + it('should set tabindex 0 on first message after removing the one that had it previously', async () => { + const messages = messageList.querySelectorAll('vaadin-message'); + const thirdMessage = messages[2]; + + // Click on third item to give it tabindex=0 + thirdMessage.dispatchEvent(new CustomEvent('mousedown', { composed: true, bubbles: true })); + thirdMessage.dispatchEvent(new CustomEvent('focus', { composed: true, bubbles: true })); + thirdMessage.dispatchEvent(new CustomEvent('mouseup', { composed: true, bubbles: true })); + + // Set message list to shorter than three items, so that tabIndex=0 can't be maintained on third item + messageList.items = [ + { + text: 'This is a new list', + time: '2:35 PM', + user: { + name: 'Steve Mops', + abbr: 'SM', + colorIndex: 2, + }, }, - }, - { - text: 'With two items', - time: '2:35 PM', - user: { - name: 'Steve Mops', - abbr: 'SM', - colorIndex: 2, + { + text: 'With two items', + time: '2:35 PM', + user: { + name: 'Steve Mops', + abbr: 'SM', + colorIndex: 2, + }, }, - }, - ]; - await nextRender(messageList); - const firstMessage = messages[0]; - // Verify that the first item got the new tabIndex=0. - expect(firstMessage.tabIndex).to.be.equal(0); - }); + ]; + await nextRender(messageList); + const firstMessage = messages[0]; + // Verify that the first item got the new tabIndex=0. + expect(firstMessage.tabIndex).to.be.equal(0); + }); - it('should preserve tabindex when increasing items count', async () => { - const secondMessage = messageList.shadowRoot.querySelectorAll('vaadin-message')[1]; - mousedown(secondMessage); - focusin(secondMessage); - messageList.items = [ - ...messageList.items, - { - text: 'A new message arrives!', - time: '2:35 PM', - user: { - name: 'Steve Mops', - abbr: 'SM', - colorIndex: 2, + it('should preserve tabindex when adding new messages', async () => { + const secondMessage = messageList.querySelectorAll('vaadin-message')[1]; + mousedown(secondMessage); + focusin(secondMessage); + messageList.items = [ + ...messageList.items, + { + text: 'A new message arrives!', + time: '2:35 PM', + user: { + name: 'Steve Mops', + abbr: 'SM', + colorIndex: 2, + }, }, - }, - ]; - await nextRender(messageList); - const messages = messageList.shadowRoot.querySelectorAll('vaadin-message'); - messages.forEach((msg, idx) => { - expect(msg.tabIndex).to.equal(idx === 1 ? 0 : -1); + ]; + await nextRender(messageList); + const messages = messageList.querySelectorAll('vaadin-message'); + messages.forEach((msg, idx) => { + expect(msg.tabIndex).to.equal(idx === 1 ? 0 : -1); + }); }); - }); - it('should preserve tabindex when decreasing items count if possible', async () => { - const secondMessage = messageList.shadowRoot.querySelectorAll('vaadin-message')[1]; - mousedown(secondMessage); - focusin(secondMessage); - - // Set message list to two items - messageList.items = [ - { - text: 'This is a new list', - time: '2:35 PM', - user: { - name: 'Steve Mops', - abbr: 'SM', - colorIndex: 2, + it('should preserve tabindex when removing messages if possible', async () => { + const secondMessage = messageList.querySelectorAll('vaadin-message')[1]; + mousedown(secondMessage); + focusin(secondMessage); + + // Set message list to two items + messageList.items = [ + { + text: 'This is a new list', + time: '2:35 PM', + user: { + name: 'Steve Mops', + abbr: 'SM', + colorIndex: 2, + }, }, - }, - { - text: 'With two items', - time: '2:35 PM', - user: { - name: 'Steve Mops', - abbr: 'SM', - colorIndex: 2, + { + text: 'With two items', + time: '2:35 PM', + user: { + name: 'Steve Mops', + abbr: 'SM', + colorIndex: 2, + }, }, - }, - ]; + ]; - await nextRender(messageList); - const messages = messageList.shadowRoot.querySelectorAll('vaadin-message'); - // Verify that the second item got the new tabIndex=0. - expect(messages[0].tabIndex).to.be.equal(-1); - expect(messages[1].tabIndex).to.be.equal(0); + await nextRender(messageList); + const messages = messageList.querySelectorAll('vaadin-message'); + // Verify that the second item got the new tabIndex=0. + expect(messages[0].tabIndex).to.be.equal(-1); + expect(messages[1].tabIndex).to.be.equal(0); + }); }); }); @@ -256,7 +285,7 @@ describe('message-list', () => { beforeEach(async () => { messageList.items = messages; await nextRender(messageList); - messageElements = messageList.shadowRoot.querySelectorAll('vaadin-message'); + messageElements = messageList.querySelectorAll('vaadin-message'); message = messageElements[1]; }); @@ -304,7 +333,7 @@ describe('message-list', () => { beforeEach(async () => { messageList.items = messages; await nextRender(messageList); - messageElements = messageList.shadowRoot.querySelectorAll('vaadin-message'); + messageElements = messageList.querySelectorAll('vaadin-message'); }); it('should set tabindex on the next message on "arrow-down" keydown', () => { diff --git a/packages/message-list/test/visual/lumo/message-list.test.js b/packages/message-list/test/visual/lumo/message-list.test.js index 78f3b4733c2..71066650d01 100644 --- a/packages/message-list/test/visual/lumo/message-list.test.js +++ b/packages/message-list/test/visual/lumo/message-list.test.js @@ -55,7 +55,7 @@ describe('message-list', () => { }); it('focused', async () => { - element.shadowRoot.querySelectorAll('vaadin-message')[0].focus(); + element.querySelectorAll('vaadin-message')[0].focus(); await sendKeys({ press: 'ArrowDown' }); await visualDiff(div, `${dir}-focused`); }); diff --git a/packages/message-list/test/visual/material/message-list.test.js b/packages/message-list/test/visual/material/message-list.test.js index e6f9a571672..d9261e2d89d 100644 --- a/packages/message-list/test/visual/material/message-list.test.js +++ b/packages/message-list/test/visual/material/message-list.test.js @@ -55,7 +55,7 @@ describe('message-list', () => { }); it('focused', async () => { - element.shadowRoot.querySelectorAll('vaadin-message')[0].focus(); + element.querySelectorAll('vaadin-message')[0].focus(); await sendKeys({ press: 'ArrowDown' }); await visualDiff(div, `${dir}-focused`); });