From 22593642d9c5adc80caea484e618ea0df97ee954 Mon Sep 17 00:00:00 2001
From: Sangeetha Babu <sangeetha9223@gmail.com>
Date: Fri, 2 Feb 2024 23:20:45 +0530
Subject: [PATCH] fix(data-table): Sorting and expansion doesn't work together
 (#11416)

### Related Ticket(s)

Closes #11178 #11481

### Description

Sorting and Expansion option works individually, but sorting is not working when used along with expansion option in Data Table.
Checking for `is-sortable` attribute during initial load seems to fix this issue.

Selection option is enabled for Data table by default without adding the is-selectable attribute to the table element.

### Changelog

**New**

- A function to handle the sorting action is added.

**Changed**

- `is-sortable` attribute was being checked only for changeAttribute cases, this is updated to check on initial load too.
- Added a condition to check if is-selectable attribute is added to the Data table component

<!-- React and Web Component deploy previews are enabled by default. -->
<!-- To enable additional available deploy previews, apply the following -->
<!-- labels for the corresponding package: -->
<!-- *** "test: e2e": Codesandbox examples and e2e integration tests -->
<!-- *** "package: services": Services -->
<!-- *** "package: utilities": Utilities -->
<!-- *** "RTL": React / Web Components (RTL) -->
<!-- *** "feature flag": React / Web Components (experimental) -->
---
 .../data-table/table-header-cell.ts           |  1 -
 .../src/components/data-table/table.ts        | 73 +++++++++++--------
 2 files changed, 44 insertions(+), 30 deletions(-)

diff --git a/packages/carbon-web-components/src/components/data-table/table-header-cell.ts b/packages/carbon-web-components/src/components/data-table/table-header-cell.ts
index c86724a006f..119b639fa3d 100644
--- a/packages/carbon-web-components/src/components/data-table/table-header-cell.ts
+++ b/packages/carbon-web-components/src/components/data-table/table-header-cell.ts
@@ -234,7 +234,6 @@ class CDSTableHeaderCell extends FocusMixin(LitElement) {
       ><slot name="slug" @slotchange="${this._handleSlugSlotChange}"></slot
     ></span> `;
   }
-
   /**
    * A selector that will return the slug item.
    */
diff --git a/packages/carbon-web-components/src/components/data-table/table.ts b/packages/carbon-web-components/src/components/data-table/table.ts
index 0fc3cd5553c..4bee2b8f1ec 100644
--- a/packages/carbon-web-components/src/components/data-table/table.ts
+++ b/packages/carbon-web-components/src/components/data-table/table.ts
@@ -664,12 +664,14 @@ class CDSTable extends HostListenerMixin(LitElement) {
     }
 
     if (changedProperties.has('isSelectable')) {
-      this._tableHeaderRow.setAttribute('selection-name', 'header');
-      this._tableRows.forEach((e, index) => {
-        if (!e.hasAttribute('selection-name')) {
-          e.setAttribute('selection-name', index);
-        }
-      });
+      if (this.isSelectable) {
+        this._tableHeaderRow.setAttribute('selection-name', 'header');
+        this._tableRows.forEach((e, index) => {
+          if (!e.hasAttribute('selection-name')) {
+            e.setAttribute('selection-name', index);
+          }
+        });
+      }
       this.headerCount++;
     }
 
@@ -677,29 +679,9 @@ class CDSTable extends HostListenerMixin(LitElement) {
       this.collator = new Intl.Collator(this.locale);
     }
     if (changedProperties.has('isSortable')) {
-      const headerCells = this.querySelectorAll(
-        (this.constructor as typeof CDSTable).selectorHeaderCell
-      );
-      headerCells.forEach((e) => {
-        (e as CDSTableHeaderCell).isSortable = this.isSortable;
-        (e as CDSTableHeaderCell).isSelectable = this.isSelectable;
-        (e as CDSTableHeaderCell).isExpandable = this.expandable;
-      });
-      const columns = [...this._tableHeaderRow.children];
-      columns.forEach((column, index) => {
-        if (
-          column.hasAttribute('sort-direction') &&
-          column.getAttribute('sort-direction') !== 'none'
-        ) {
-          const sortDirection = column.getAttribute('sort-direction');
-          const columnIndex = index;
-
-          columns.forEach(
-            (e) => e !== column && e.setAttribute('sort-direction', 'none')
-          );
-          this._handleSortAction(columnIndex, sortDirection);
-        }
-      });
+      if (this.isSortable) {
+        this._enableSortAction();
+      }
     }
 
     if (
@@ -813,6 +795,39 @@ class CDSTable extends HostListenerMixin(LitElement) {
         : html`<slot></slot>`}
     `;
   }
+
+  /**
+   * Adds isSortable value for table header cells.
+   */
+  _enableSortAction() {
+    const headerCells = this.querySelectorAll(
+      (this.constructor as typeof CDSTable).selectorHeaderCell
+    );
+    headerCells.forEach((e) => {
+      (e as CDSTableHeaderCell).isSortable = this.isSortable;
+      (e as CDSTableHeaderCell).isSelectable = this.isSelectable;
+      (e as CDSTableHeaderCell).isExpandable = this.expandable;
+    });
+    const columns = [...this._tableHeaderRow.children];
+    let sortDirection;
+    let columnIndex = -1;
+    columns.forEach((column, index) => {
+      if (
+        column.hasAttribute('sort-direction') &&
+        column.getAttribute('sort-direction') !== 'none'
+      ) {
+        sortDirection = column.getAttribute('sort-direction');
+        columnIndex = index;
+      }
+    });
+
+    columns.forEach(
+      (e, index) =>
+        index !== columnIndex && e.setAttribute('sort-direction', 'none')
+    );
+    this._handleSortAction(columnIndex, sortDirection);
+  }
+
   /* eslint-enable no-constant-condition */
 
   /**