From 72e129317ad6bbe64cd65c73dc32054c75b67c81 Mon Sep 17 00:00:00 2001 From: Serhii Kulykov Date: Fri, 15 Nov 2024 10:00:37 +0200 Subject: [PATCH] refactor!: update avatar group to not use Polymer splices API (#8145) --- .../avatar-group/src/vaadin-avatar-group.js | 111 +++++++----------- .../avatar-group/test/avatar-group.test.js | 24 ++-- .../__snapshots__/avatar-group.test.snap.js | 2 +- 3 files changed, 52 insertions(+), 85 deletions(-) diff --git a/packages/avatar-group/src/vaadin-avatar-group.js b/packages/avatar-group/src/vaadin-avatar-group.js index 87ae725cec7..65e364225a0 100644 --- a/packages/avatar-group/src/vaadin-avatar-group.js +++ b/packages/avatar-group/src/vaadin-avatar-group.js @@ -7,7 +7,6 @@ import '@vaadin/avatar/src/vaadin-avatar.js'; import './vaadin-avatar-group-menu.js'; import './vaadin-avatar-group-menu-item.js'; import './vaadin-avatar-group-overlay.js'; -import { calculateSplices } from '@polymer/polymer/lib/utils/array-splice.js'; import { afterNextRender } from '@polymer/polymer/lib/utils/render-status.js'; import { html as legacyHtml, PolymerElement } from '@polymer/polymer/polymer-element.js'; import { html, render } from 'lit'; @@ -126,6 +125,7 @@ class AvatarGroup extends ResizeMixin(OverlayClassMixin(ElementMixin(ThemableMix */ items: { type: Array, + observer: '__itemsChanged', }, /** @@ -188,17 +188,6 @@ class AvatarGroup extends ResizeMixin(OverlayClassMixin(ElementMixin(ThemableMix value: () => [], }, - /** @private */ - __maxReached: { - type: Boolean, - computed: '__computeMaxReached(items.length, maxItemsVisible)', - }, - - /** @private */ - __items: { - type: Array, - }, - /** @private */ __itemsInView: { type: Number, @@ -214,7 +203,7 @@ class AvatarGroup extends ResizeMixin(OverlayClassMixin(ElementMixin(ThemableMix _overflowItems: { type: Array, observer: '__overflowItemsChanged', - computed: '__computeOverflowItems(items.*, __itemsInView, maxItemsVisible)', + computed: '__computeOverflowItems(items, __itemsInView, maxItemsVisible)', }, /** @private */ @@ -232,13 +221,11 @@ class AvatarGroup extends ResizeMixin(OverlayClassMixin(ElementMixin(ThemableMix static get observers() { return [ - '__itemsChanged(items.splices, items.*)', - '__i18nItemsChanged(i18n.*, items.length)', + '__i18nItemsChanged(i18n, items)', '__updateAvatarsTheme(_overflow, _avatars, _theme)', - '__updateAvatars(items.*, __itemsInView, maxItemsVisible, _overflow, i18n)', - '__updateOverflowAbbr(_overflow, items.length, __itemsInView, maxItemsVisible)', - '__updateOverflowHidden(_overflow, items.length, __itemsInView, __maxReached)', - '__updateOverflowTooltip(_overflowTooltip, items.length, __itemsInView, maxItemsVisible)', + '__updateAvatars(items, __itemsInView, maxItemsVisible, _overflow, i18n)', + '__updateOverflowAvatar(_overflow, items, __itemsInView, maxItemsVisible)', + '__updateOverflowTooltip(_overflowTooltip, items, __itemsInView, maxItemsVisible)', ]; } @@ -406,12 +393,11 @@ class AvatarGroup extends ResizeMixin(OverlayClassMixin(ElementMixin(ThemableMix } /** @private */ - __updateAvatars(arr, itemsInView, maxItemsVisible, overflow) { - if (!overflow) { + __updateAvatars(items, itemsInView, maxItemsVisible, overflow) { + if (!overflow || !Array.isArray(items)) { return; } - const items = arr.base || []; const limit = this.__getLimit(items.length, itemsInView, maxItemsVisible); this.__renderAvatars(limit ? items.slice(0, limit) : items); @@ -420,28 +406,20 @@ class AvatarGroup extends ResizeMixin(OverlayClassMixin(ElementMixin(ThemableMix } /** @private */ - __computeOverflowItems(arr, itemsInView, maxItemsVisible) { - const items = arr.base || []; - const limit = this.__getLimit(items.length, itemsInView, maxItemsVisible); + __computeOverflowItems(items, itemsInView, maxItemsVisible) { + const count = Array.isArray(items) ? items.length : 0; + const limit = this.__getLimit(count, itemsInView, maxItemsVisible); return limit ? items.slice(limit) : []; } /** @private */ - __computeMaxReached(items, maxItemsVisible) { - return maxItemsVisible != null && items > this.__getMax(maxItemsVisible); - } - - /** @private */ - __updateOverflowAbbr(overflow, items, itemsInView, maxItemsVisible) { + __updateOverflowAvatar(overflow, items, itemsInView, maxItemsVisible) { if (overflow) { - overflow.abbr = `+${items - this.__getLimit(items, itemsInView, maxItemsVisible)}`; - } - } + const count = Array.isArray(items) ? items.length : 0; + const maxReached = maxItemsVisible != null && count > this.__getMax(maxItemsVisible); - /** @private */ - __updateOverflowHidden(overflow, items, itemsInView, maxReached) { - if (overflow) { - overflow.toggleAttribute('hidden', !maxReached && !(itemsInView && itemsInView < items)); + overflow.abbr = `+${count - this.__getLimit(count, itemsInView, maxItemsVisible)}`; + overflow.toggleAttribute('hidden', !maxReached && !(itemsInView && itemsInView < count)); } } @@ -460,18 +438,18 @@ class AvatarGroup extends ResizeMixin(OverlayClassMixin(ElementMixin(ThemableMix /** @private */ __updateOverflowTooltip(tooltip, items, itemsInView, maxItemsVisible) { - if (!tooltip) { + if (!tooltip || !Array.isArray(items)) { return; } - const limit = this.__getLimit(items, itemsInView, maxItemsVisible); + const limit = this.__getLimit(items.length, itemsInView, maxItemsVisible); if (limit == null) { return; } const result = []; - for (let i = limit; i < items; i++) { - const item = this.items[i]; + for (let i = limit; i < items.length; i++) { + const item = items[i]; if (item) { result.push(item.name || item.abbr || 'anonymous'); } @@ -500,35 +478,32 @@ class AvatarGroup extends ResizeMixin(OverlayClassMixin(ElementMixin(ThemableMix } /** @private */ - __itemsChanged(splices, itemsChange) { - const items = itemsChange.base; + __itemsChanged(items, oldItems) { this.__setItemsInView(); - // Mutation using group.splice('items') - if (splices && Array.isArray(splices.indexSplices)) { - splices.indexSplices.forEach((mutation) => { - this.__announceItemsChange(items, mutation); - }); - } else if (Array.isArray(items) && Array.isArray(this.__oldItems)) { - // Mutation using group.set('items') - const diff = calculateSplices(items, this.__oldItems); - diff.forEach((mutation) => { - this.__announceItemsChange(items, mutation); - }); + let added = []; + let removed = []; + + const hasNewItems = Array.isArray(items); + const hasOldItems = Array.isArray(oldItems); + + if (hasOldItems) { + removed = oldItems.filter((item) => hasNewItems && !items.includes(item)); + } + + if (hasNewItems) { + added = items.filter((item) => hasOldItems && !oldItems.includes(item)); } - this.__oldItems = items; + this.__announceItemsChange(added, removed); } /** @private */ - __announceItemsChange(items, mutation) { - const { addedCount, index, removed } = mutation; + __announceItemsChange(added, removed) { let addedMsg = []; let removedMsg = []; - if (addedCount) { - addedMsg = items - .slice(index, index + addedCount) - .map((user) => this.__getMessage(user, this.i18n.joined || '{user} joined')); + if (added) { + addedMsg = added.map((user) => this.__getMessage(user, this.i18n.joined || '{user} joined')); } if (removed) { @@ -543,15 +518,15 @@ class AvatarGroup extends ResizeMixin(OverlayClassMixin(ElementMixin(ThemableMix /** @private */ __i18nItemsChanged(i18n, items) { - const { base } = i18n; - if (base && base.activeUsers) { - const field = items === 1 ? 'one' : 'many'; - if (base.activeUsers[field]) { - this.setAttribute('aria-label', base.activeUsers[field].replace('{count}', items || 0)); + if (i18n && i18n.activeUsers) { + const count = Array.isArray(items) ? items.length : 0; + const field = count === 1 ? 'one' : 'many'; + if (i18n.activeUsers[field]) { + this.setAttribute('aria-label', i18n.activeUsers[field].replace('{count}', count || 0)); } this._avatars.forEach((avatar) => { - avatar.i18n = base; + avatar.i18n = i18n; }); } } diff --git a/packages/avatar-group/test/avatar-group.test.js b/packages/avatar-group/test/avatar-group.test.js index ab24172d802..84c66ca4a03 100644 --- a/packages/avatar-group/test/avatar-group.test.js +++ b/packages/avatar-group/test/avatar-group.test.js @@ -163,7 +163,7 @@ describe('avatar-group', () => { it('should hide overflow avatar when items property is changed', async () => { group.maxItemsVisible = 2; await nextRender(group); - group.splice('items', 1, 3); + group.items = group.items.slice(0, 2); await nextRender(group); expect(group._overflow.hasAttribute('hidden')).to.be.true; }); @@ -224,7 +224,7 @@ describe('avatar-group', () => { }); it('should always show at least two avatars', async () => { - group.set('items', group.items.slice(0, 2)); + group.items = group.items.slice(0, 2); group.style.width = '50px'; await onceResized(group); const items = group.querySelectorAll('vaadin-avatar:not([hidden])'); @@ -588,7 +588,7 @@ describe('avatar-group', () => { }); it('should announce when adding single item', () => { - group.splice('items', 2, 0, { name: 'DD' }); + group.items = [...group.items, { name: 'DD' }]; clock.tick(150); @@ -596,7 +596,7 @@ describe('avatar-group', () => { }); it('should announce when removing single item', () => { - group.splice('items', 2, 1); + group.items = group.items.slice(0, 2); clock.tick(150); @@ -604,7 +604,7 @@ describe('avatar-group', () => { }); it('should announce when adding multiple items', () => { - group.splice('items', 2, 0, { name: 'DD' }, { name: 'EE' }); + group.items = [...group.items, { name: 'DD' }, { name: 'EE' }]; clock.tick(150); @@ -612,7 +612,7 @@ describe('avatar-group', () => { }); it('should announce when removing multiple items', () => { - group.splice('items', 1, 2); + group.items = group.items.slice(0, 1); clock.tick(150); @@ -620,7 +620,7 @@ describe('avatar-group', () => { }); it('should announce when adding and removing single item', () => { - group.splice('items', 2, 1, { name: 'DD' }); + group.items = [...group.items.slice(0, 2), { name: 'DD' }]; clock.tick(150); @@ -628,20 +628,12 @@ describe('avatar-group', () => { }); it('should announce when adding and removing multiple items', () => { - group.splice('items', 1, 2, { name: 'DD' }, { name: 'EE' }); + group.items = [...group.items.slice(0, 1), { name: 'DD' }, { name: 'EE' }]; clock.tick(150); expect(region.textContent).to.equal('BB left, CC left, DD joined, EE joined'); }); - - it('should announce when the items property is reset', () => { - group.set('items', [group.items[0]]); - - clock.tick(150); - - expect(region.textContent).to.equal('BB left, CC left'); - }); }); }); diff --git a/packages/avatar-group/test/dom/__snapshots__/avatar-group.test.snap.js b/packages/avatar-group/test/dom/__snapshots__/avatar-group.test.snap.js index 2371b9b30cf..bd89d0462b9 100644 --- a/packages/avatar-group/test/dom/__snapshots__/avatar-group.test.snap.js +++ b/packages/avatar-group/test/dom/__snapshots__/avatar-group.test.snap.js @@ -4,7 +4,7 @@ export const snapshots = {}; snapshots["vaadin-avatar-group default"] = `