fix algo and state management for colors

microsoft · Jun 13, 2024 · cb488e2 · cb488e2
1 parent 3c9d222
commit cb488e2
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 16 deletions.
diff --git a/packages/web-components/src/avatar/avatar.spec.ts b/packages/web-components/src/avatar/avatar.spec.ts
@@ -155,7 +155,7 @@ test.describe('Avatar Component', () => {
   test('should prioritize color derivation from colorId over name when set to "colorful"', async () => {
     await root.evaluate(node => {
       node.innerHTML = /* html */ `
-        <fluent-avatar color-id="pumpkin" name="John Doe" color="colorful"></fluent-avatar>
+        <fluent-avatar color-id="pumpkin" name="Steve Smith" color="colorful"></fluent-avatar>
       `;
     });
 

diff --git a/packages/web-components/src/avatar/avatar.ts b/packages/web-components/src/avatar/avatar.ts
@@ -1,4 +1,4 @@
-import { attr, FASTElement, nullableNumberConverter, Observable } from '@microsoft/fast-element';
+import { attr, FASTElement, nullableNumberConverter, observable, Observable, Updates } from '@microsoft/fast-element';
 import { getInitials } from '../utils/get-initials.js';
 import { toggleState } from '../utils/element-internals.js';
 import {
@@ -104,7 +104,7 @@ export class Avatar extends FASTElement {
    * HTML Attribute: color
    */
   @attr
-  public color?: AvatarColor = 'neutral';
+  public color?: AvatarColor | undefined;
 
   /**
    * Specify a string to be used instead of the name, to determine which color to use when color="colorful".
@@ -113,6 +113,11 @@ export class Avatar extends FASTElement {
   @attr({ attribute: 'color-id' })
   public colorId?: AvatarNamedColor | undefined;
 
+  /**
+   * Holds the current color state
+   */
+  private currentColor: string | undefined;
+
   constructor() {
     super();
 
@@ -124,9 +129,7 @@ export class Avatar extends FASTElement {
 
     Observable.getNotifier(this).subscribe(this);
 
-    Object.keys(this.$fastController.definition.attributeLookup).forEach(key => {
-      this.handleChange(this, key);
-    });
+    this.generateColor();
   }
 
   public disconnectedCallback(): void {
@@ -142,8 +145,13 @@ export class Avatar extends FASTElement {
    * @param propertyName - the property name being changed
    */
   public handleChange(source: any, propertyName: string) {
-    if (propertyName === ('color' || 'colorId')) {
-      this.generateColor();
+    switch (propertyName) {
+      case 'color':
+      case 'colorId':
+        this.generateColor();
+        break;
+      default:
+        break;
     }
   }
 
@@ -152,16 +160,19 @@ export class Avatar extends FASTElement {
    * @internal
    */
   public generateColor(): void {
-    if (!this.color) {
-      return;
-    }
+    const colorful: boolean = this.color === AvatarColor.colorful;
+    const prev = this.currentColor;
+
+    toggleState(this.elementInternals, `${prev}`, false);
 
-    const color =
-      this.color === AvatarColor.colorful
-        ? (Avatar.colors[getHashCode(this.colorId ?? this.name ?? '') % Avatar.colors.length] as AvatarColor)
-        : this.color;
+    this.currentColor =
+      colorful && this.colorId
+        ? this.colorId
+        : colorful
+        ? (Avatar.colors[getHashCode(this.name ?? '') % Avatar.colors.length] as AvatarColor)
+        : this.color ?? AvatarColor.neutral;
 
-    toggleState(this.elementInternals, color, !!this.color);
+    toggleState(this.elementInternals, `${this.currentColor}`, true);
   }
 
   /**