refactor!: update avatar group to not use Polymer splices API (#8145)

Author: web-padawan

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;
       });
     }
   }

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,60 +588,52 @@ 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);
 
       expect(region.textContent).to.equal('DD joined');
     });
 
     it('should announce when removing single item', () => {
-      group.splice('items', 2, 1);
+      group.items = group.items.slice(0, 2);
 
       clock.tick(150);
 
       expect(region.textContent).to.equal('CC left');
     });
 
     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);
 
       expect(region.textContent).to.equal('DD joined, EE joined');
     });
 
     it('should announce when removing multiple items', () => {
-      group.splice('items', 1, 2);
+      group.items = group.items.slice(0, 1);
 
       clock.tick(150);
 
       expect(region.textContent).to.equal('BB left, CC left');
     });
 
     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);
 
       expect(region.textContent).to.equal('CC left, DD joined');
     });
 
     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');
-    });
   });
 });
 

packages/avatar-group/test/dom/__snapshots__/avatar-group.test.snap.js

@@ -4,7 +4,7 @@ export const snapshots = {};
 snapshots["vaadin-avatar-group default"] = 
 `<vaadin-avatar-group aria-label="Currently 0 active users">
   <vaadin-avatar
-    abbr="+NaN"
+    abbr="+0"
     aria-expanded="false"
     aria-haspopup="menu"
     hidden=""