From 64c1a9b8f320cb4d215734991c8b5c47d1dc3ee3 Mon Sep 17 00:00:00 2001
From: Amir Ashkan Baghdoust <amir-ashkan.baghdoust@telekom.de>
Date: Wed, 16 Oct 2024 18:35:54 +0200
Subject: [PATCH] refactor(segment): improves loading of the component and
 solves nested size issues

---
 .../src/components/segment/segment.tsx        |  9 +--
 .../segmented-button.spec.ts.snap             |  6 +-
 .../segmented-button/segmented-button.tsx     | 72 ++++++++++---------
 .../components/src/html/segment-button.html   | 48 +++++++++++++
 4 files changed, 90 insertions(+), 45 deletions(-)
 create mode 100644 packages/components/src/html/segment-button.html

diff --git a/packages/components/src/components/segment/segment.tsx b/packages/components/src/components/segment/segment.tsx
index c2daf67c0c..d6aed90308 100644
--- a/packages/components/src/components/segment/segment.tsx
+++ b/packages/components/src/components/segment/segment.tsx
@@ -38,7 +38,7 @@ export class Segment {
   /** (optional) If `true`, the segment is disabled */
   @Prop() disabled?: boolean = false;
   /** (optional) segment's id */
-  @Prop({ reflect: true, mutable: true }) segmentId?: string;
+  @Prop({ reflect: true }) segmentId?: string = 'segment-' + i++;
   /** (optional) aria-label attribute needed for icon-only segments */
   @Prop() ariaLabelSegment: string;
   /** (optional) Segment width set to ensure that all segments have the same width */
@@ -85,12 +85,7 @@ export class Segment {
     this.focusableElement.focus();
   }
 
-  componentWillLoad() {
-    if (this.segmentId == null) {
-      this.segmentId = 'segment-' + i++;
-    }
-  }
-  componentDidUpdate() {
+  componentWillUpdate() {
     this.handleIcon();
   }
 
diff --git a/packages/components/src/components/segmented-button/__snapshots__/segmented-button.spec.ts.snap b/packages/components/src/components/segmented-button/__snapshots__/segmented-button.spec.ts.snap
index 41c565086d..82e5bc5170 100644
--- a/packages/components/src/components/segmented-button/__snapshots__/segmented-button.spec.ts.snap
+++ b/packages/components/src/components/segmented-button/__snapshots__/segmented-button.spec.ts.snap
@@ -3,7 +3,7 @@
 exports[`SegmentedButton should match selected button snapshot 1`] = `
 <scale-segmented-button>
   <mock:shadow-root>
-    <div aria-label="segment button with 2" class="segmented-button segmented-button--small" part="segmented-button small" role="group" style="grid-template-columns: repeat(2, 0px);">
+    <div aria-label="segment button with 2" class="segmented-button segmented-button--small" part="segmented-button small" role="group" style="grid-template-columns: repeat(2, auto);">
       <slot></slot>
     </div>
   </mock:shadow-root>
@@ -19,7 +19,7 @@ exports[`SegmentedButton should match selected button snapshot 1`] = `
 exports[`SegmentedButton should match standard snapshot 1`] = `
 <scale-segmented-button>
   <mock:shadow-root>
-    <div aria-label="segment button with 4" class="segmented-button segmented-button--small" part="segmented-button small" role="group" style="grid-template-columns: repeat(4, 14px);">
+    <div aria-label="segment button with 4" class="segmented-button segmented-button--small" part="segmented-button small" role="group" style="grid-template-columns: repeat(4, auto);">
       <slot></slot>
     </div>
   </mock:shadow-root>
@@ -36,4 +36,4 @@ exports[`SegmentedButton should match standard snapshot 1`] = `
     Label
   </scale-segment>
 </scale-segmented-button>
-`;
+`;
\ No newline at end of file
diff --git a/packages/components/src/components/segmented-button/segmented-button.tsx b/packages/components/src/components/segmented-button/segmented-button.tsx
index b93fea17c1..bc104f9523 100644
--- a/packages/components/src/components/segmented-button/segmented-button.tsx
+++ b/packages/components/src/components/segmented-button/segmented-button.tsx
@@ -52,7 +52,7 @@ export class SegmentedButton {
   /** (optional) Allow more than one button to be selected */
   @Prop() multiSelect: boolean = false;
   /** (optional) the index of the selected segment */
-  @Prop() selectedIndex?: number;
+  @Prop({ mutable: true }) selectedIndex?: number;
   /** (optional) If `true`, the button is disabled */
   @Prop({ reflect: true }) disabled?: boolean = false;
   /** (optional) If `true`, expand to container width */
@@ -119,36 +119,36 @@ export class SegmentedButton {
     });
   }
 
-  componentDidLoad() {
+  componentWillLoad() {
     const tempState: SegmentStatus[] = [];
     const segments = this.getAllSegments();
     this.slottedSegments = segments.length;
-    const longestButtonWidth = this.getLongestButtonWidth();
-    segments.forEach((segment) => {
-      this.position++;
+    segments.forEach((segment, i) => {
       tempState.push({
         id: segment.getAttribute('segment-id') || segment.segmentId,
         selected: segment.hasAttribute('selected') || segment.selected,
       });
-      segment.setAttribute('position', this.position.toString());
+      segment.setAttribute('position', `${i + 1}`);
       segment.setAttribute(
         'aria-description-translation',
         '$position $selected'
       );
     });
+    this.setState(tempState);
+    this.selectedIndex = this.getSelectedIndex();
+  }
+  componentDidLoad() {
+    const longestButtonWidth = this.getLongestButtonWidth();
     if (!this.fullWidth) {
-      this.container.style.gridTemplateColumns = `repeat(${
-        this.hostElement.children.length
-      }, ${Math.ceil(longestButtonWidth)}px)`;
+      this.container.style.gridTemplateColumns = longestButtonWidth
+        ? `repeat(${this.hostElement.children.length}, ${Math.ceil(
+            longestButtonWidth
+          )}px)`
+        : `repeat(${this.hostElement.children.length}, auto)`;
     } else {
       this.container.style.display = 'flex';
     }
-
-    this.selectedIndex = this.getSelectedIndex();
     this.propagatePropsToChildren();
-    this.position = 0;
-    this.status = tempState;
-    this.setState(tempState);
   }
 
   componentWillUpdate() {
@@ -195,27 +195,29 @@ export class SegmentedButton {
   // all segmented buttons should have the same width, based on the largest one
   getLongestButtonWidth() {
     let tempWidth = 0;
-    Array.from(this.hostElement.children).forEach((child) => {
-      const selected = child.hasAttribute('selected');
-      const iconOnly = child.hasAttribute('icon-only');
-      const checkmark =
-        this.size === 'small'
-          ? CHECKMARK_WIDTH_SMALL
-          : this.size === 'medium'
-          ? CHECKMARK_WIDTH_MEDIUM
-          : CHECKMARK_WIDTH_LARGE;
-      if (selected || iconOnly) {
-        tempWidth =
-          child.getBoundingClientRect().width > tempWidth
-            ? child.getBoundingClientRect().width
-            : tempWidth;
-      } else {
-        tempWidth =
-          child.getBoundingClientRect().width + checkmark > tempWidth
-            ? child.getBoundingClientRect().width + checkmark
-            : tempWidth;
-      }
-    });
+    Array.from(this.hostElement.children)
+      .filter((child) => child.getBoundingClientRect().width)
+      .forEach((child) => {
+        const selected = child.hasAttribute('selected');
+        const iconOnly = child.hasAttribute('icon-only');
+        const checkmark =
+          this.size === 'small'
+            ? CHECKMARK_WIDTH_SMALL
+            : this.size === 'medium'
+            ? CHECKMARK_WIDTH_MEDIUM
+            : CHECKMARK_WIDTH_LARGE;
+        if (selected || iconOnly) {
+          tempWidth =
+            child.getBoundingClientRect().width > tempWidth
+              ? child.getBoundingClientRect().width
+              : tempWidth;
+        } else {
+          tempWidth =
+            child.getBoundingClientRect().width + checkmark > tempWidth
+              ? child.getBoundingClientRect().width + checkmark
+              : tempWidth;
+        }
+      });
     return tempWidth;
   }
 
diff --git a/packages/components/src/html/segment-button.html b/packages/components/src/html/segment-button.html
new file mode 100644
index 0000000000..00391810bb
--- /dev/null
+++ b/packages/components/src/html/segment-button.html
@@ -0,0 +1,48 @@
+<!DOCTYPE html>
+<html dir="ltr" lang="en">
+  <head>
+    <meta charset="utf-8" />
+    <meta
+      name="viewport"
+      content="width=device-width, initial-scale=1.0, minimum-scale=1.0, maximum-scale=5.0"
+    />
+    <title>Loading spinners</title>
+
+    <script type="module" src="build/scale-components.esm.js"></script>
+    <script nomodule src="build/scale-components.js"></script>
+    <link rel="stylesheet" href="build/scale-components.css" />
+    <style>
+      section {
+        padding: 1rem;
+      }
+      .bg-black {
+        background: black;
+        color: white;
+      }
+    </style>
+  </head>
+
+  <body>
+    <scale-collapsible>
+      <div slot="heading">Open Me for Disaster</div>
+      <scale-segmented-button>
+        <scale-segment selected>Apple</scale-segment>
+        <scale-segment>One+</scale-segment>
+        <scale-segment>Samsung</scale-segment>
+        <scale-segment>Huawei</scale-segment>
+      </scale-segmented-button>
+    </scale-collapsible>
+
+    <scale-segmented-button>
+      <scale-segment>Apple</scale-segment>
+      <scale-segment>One+</scale-segment>
+      <scale-segment>Samsung</scale-segment>
+      <scale-segment>Huawei</scale-segment>
+    </scale-segmented-button>
+
+    <scale-segmented-button>
+      <scale-segment selected>Label</scale-segment>
+      <scale-segment selected>Label</scale-segment>
+    </scale-segmented-button>
+  </body>
+</html>
\ No newline at end of file