fix: do not auto-select first element in detailed list (#400)

* fix: do not auto-select first element in detailed list

* test: update screenshots

* fix: add early returns to improve readability

Author: avi-alpert

src/__test__/components/detailed-list/detailed-list.spec.ts

@@ -574,6 +574,7 @@ describe('DetailedListWrapper Component', () => {
     it('should get target element', () => {
       detailedListWrapper = new DetailedListWrapper({ detailedList: basicDetailedList });
       document.body.appendChild(detailedListWrapper.render);
+      detailedListWrapper.changeTarget('down', false, true);
 
       const targetElement = detailedListWrapper.getTargetElement();
       expect(targetElement).toBeDefined();
@@ -615,6 +616,9 @@ describe('DetailedListWrapper Component', () => {
       detailedListWrapper = new DetailedListWrapper({ detailedList: basicDetailedList });
       document.body.appendChild(detailedListWrapper.render);
 
+      // First select the first item
+      detailedListWrapper.changeTarget('down');
+
       // Go up from first item (should wrap to last)
       detailedListWrapper.changeTarget('up', false);
       const targetElement = detailedListWrapper.getTargetElement();
@@ -625,7 +629,9 @@ describe('DetailedListWrapper Component', () => {
       detailedListWrapper = new DetailedListWrapper({ detailedList: basicDetailedList });
       document.body.appendChild(detailedListWrapper.render);
 
-      // Move to last item first
+      // First select an item
+      detailedListWrapper.changeTarget('down');
+      // Move to last item
       detailedListWrapper.changeTarget('down');
       // Then go down (should wrap to first)
       detailedListWrapper.changeTarget('down', false);
@@ -637,6 +643,9 @@ describe('DetailedListWrapper Component', () => {
       detailedListWrapper = new DetailedListWrapper({ detailedList: basicDetailedList });
       document.body.appendChild(detailedListWrapper.render);
 
+      // First select the first item
+      detailedListWrapper.changeTarget('down');
+
       // Go up with snap (should stay at first)
       detailedListWrapper.changeTarget('up', true);
       let targetElement = detailedListWrapper.getTargetElement();

src/components/detailed-list/detailed-list-item.ts

@@ -17,6 +17,7 @@ export interface DetailedListItemWrapperProps {
   onSelect?: (detailedListItem: DetailedListItem) => void;
   onClick?: (detailedListItem: DetailedListItem) => void;
   onActionClick?: (action: ChatItemButton, detailedListItem?: DetailedListItem) => void;
+  onShowActionMenuOverlay?: () => void;
   selectable?: boolean;
   clickable?: boolean;
   textDirection?: 'row' | 'column';
@@ -201,6 +202,7 @@ export class DetailedListItemWrapper {
   };
 
   private readonly showActionMenuOverlay = (listItem?: DetailedListItem): void => {
+    this.props.onShowActionMenuOverlay?.();
     this.actionMenuOverlay = new Overlay({
       background: true,
       closeOnOutsideClick: true,

src/components/detailed-list/detailed-list.ts

@@ -37,7 +37,7 @@ export class DetailedListWrapper {
     element: ExtendedHTMLElement;
   }> = [];
 
-  private activeTargetElementIndex: number = 0;
+  private activeTargetElementIndex: number = -1;
   private allSelectableDetailedListElements: DetailedListItemWrapper[] = [];
   constructor (props: DetailedListWrapperProps) {
     this.props = props;
@@ -221,7 +221,6 @@ export class DetailedListWrapper {
         ]
       });
     });
-    this.allSelectableDetailedListElements[0]?.setFocus(true, true);
     return groups ?? [ '' ];
   };
 
@@ -231,6 +230,7 @@ export class DetailedListWrapper {
         listItem: detailedListItem,
         onSelect: this.props.onItemSelect,
         onClick: this.props.onItemClick,
+        onShowActionMenuOverlay: () => { this.setFocus(detailedListItem); },
         onActionClick: this.props.onItemActionClick,
         selectable: this.props.detailedList.selectable !== false && this.props.detailedList.selectable !== 'clickable',
         clickable: this.props.detailedList.selectable === 'clickable',
@@ -244,38 +244,64 @@ export class DetailedListWrapper {
     });
   };
 
-  public readonly changeTarget = (direction: 'up' | 'down', snapOnLastAndFirst?: boolean, scrollIntoView?: boolean): void => {
-    if (this.allSelectableDetailedListElements.length > 0) {
-      let nextElementIndex: number = this.activeTargetElementIndex;
-      if (direction === 'up') {
-        if (nextElementIndex > 0) {
-          nextElementIndex--;
-        } else if (snapOnLastAndFirst !== true) {
-          nextElementIndex = this.allSelectableDetailedListElements.length - 1;
-        } else {
-          nextElementIndex = 0;
-        }
-      } else {
-        if (nextElementIndex < this.allSelectableDetailedListElements.length - 1) {
-          nextElementIndex++;
-        } else if (snapOnLastAndFirst !== true) {
-          nextElementIndex = 0;
-        } else {
-          nextElementIndex = this.allSelectableDetailedListElements.length - 1;
-        }
-      }
+  public readonly changeTarget = (
+    direction: 'up' | 'down',
+    snapOnLastAndFirst?: boolean,
+    scrollIntoView?: boolean
+  ): void => {
+    if (this.allSelectableDetailedListElements.length === 0) return;
+
+    const lastIndex = this.allSelectableDetailedListElements.length - 1;
+    let nextElementIndex = this.activeTargetElementIndex;
+
+    // Handle initial selection when no item is selected
+    if (nextElementIndex === -1) {
+      nextElementIndex = direction === 'up' ? lastIndex : 0;
+      this.setFocusByIndex(nextElementIndex, scrollIntoView);
+      return;
+    }
+
+    // Calculate next index based on direction
+    if (direction === 'up') {
+      nextElementIndex = nextElementIndex > 0
+        ? nextElementIndex - 1
+        : (snapOnLastAndFirst === true ? 0 : lastIndex);
+    } else {
+      nextElementIndex = nextElementIndex < lastIndex
+        ? nextElementIndex + 1
+        : (snapOnLastAndFirst === true ? lastIndex : 0);
+    }
+
+    this.setFocusByIndex(nextElementIndex, scrollIntoView);
+  };
+
+  private readonly setFocus = (detailedListItem: DetailedListItem): void => {
+    // Only remove focus from current item if one is selected
+    if (this.activeTargetElementIndex >= 0) {
+      this.allSelectableDetailedListElements[this.activeTargetElementIndex].setFocus(false, false);
+    }
+    const selectedItemIndex = this.allSelectableDetailedListElements.findIndex(item => item.getItem().id === detailedListItem.id);
 
+    this.activeTargetElementIndex = selectedItemIndex;
+    this.allSelectableDetailedListElements[this.activeTargetElementIndex].setFocus(true, false);
+  };
+
+  private readonly setFocusByIndex = (index: number, scrollIntoView?: boolean): void => {
+    // Only remove focus from current item if one is selected
+    if (this.activeTargetElementIndex >= 0) {
       this.allSelectableDetailedListElements[this.activeTargetElementIndex].setFocus(false, scrollIntoView === true);
-      this.activeTargetElementIndex = nextElementIndex;
-      this.allSelectableDetailedListElements[this.activeTargetElementIndex].setFocus(true, scrollIntoView === true);
     }
+
+    this.activeTargetElementIndex = index;
+    this.allSelectableDetailedListElements[this.activeTargetElementIndex].setFocus(true, scrollIntoView === true);
   };
 
   public readonly getTargetElement = (): DetailedListItem | null => {
-    if (this.allSelectableDetailedListElements.length > 0) {
-      return this.allSelectableDetailedListElements[Math.max(this.activeTargetElementIndex, 0)].getItem();
+    if (this.allSelectableDetailedListElements.length === 0 || this.activeTargetElementIndex < 0) {
+      return null;
     }
-    return null;
+
+    return this.allSelectableDetailedListElements[this.activeTargetElementIndex].getItem();
   };
 
   public readonly update = (detailedList: DetailedList, preserveScrollPosition?: boolean): void => {
@@ -310,7 +336,7 @@ export class DetailedListWrapper {
       // Clear and recreate the list structure
       this.detailedListItemsBlockData = [];
       this.detailedListItemGroupsContainer.clear();
-      this.activeTargetElementIndex = 0;
+      this.activeTargetElementIndex = -1;
       this.allSelectableDetailedListElements = [];
       this.props.detailedList.list = detailedList.list;