From 64c1a9b8f320cb4d215734991c8b5c47d1dc3ee3 Mon Sep 17 00:00:00 2001 From: Amir Ashkan Baghdoust Date: Wed, 16 Oct 2024 18:35:54 +0200 Subject: [PATCH 1/5] 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`] = ` -
+
@@ -19,7 +19,7 @@ exports[`SegmentedButton should match selected button snapshot 1`] = ` exports[`SegmentedButton should match standard snapshot 1`] = ` -
+
@@ -36,4 +36,4 @@ exports[`SegmentedButton should match standard snapshot 1`] = ` Label -`; +`; \ 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 @@ + + + + + + Loading spinners + + + + + + + + + +
Open Me for Disaster
+ + Apple + One+ + Samsung + Huawei + + + + + Apple + One+ + Samsung + Huawei + + + + Label + Label + + + \ No newline at end of file From 6199c384f209a6c4ecd34c9a5bbc2b21132354d7 Mon Sep 17 00:00:00 2001 From: Amir Ashkan Baghdoust Date: Wed, 16 Oct 2024 19:12:45 +0200 Subject: [PATCH 2/5] refactor(segment): fix missing newline at end of file in segment-button.html --- packages/components/src/html/segment-button.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/src/html/segment-button.html b/packages/components/src/html/segment-button.html index 00391810bb..bf89b5ba49 100644 --- a/packages/components/src/html/segment-button.html +++ b/packages/components/src/html/segment-button.html @@ -45,4 +45,4 @@ Label - \ No newline at end of file + From c8838479949ce195f79ca487208d4635855c65e0 Mon Sep 17 00:00:00 2001 From: Amir Ashkan Baghdoust Date: Wed, 16 Oct 2024 19:13:10 +0200 Subject: [PATCH 3/5] refactor(segment): adds missing docs --- .../src/components/segment/readme.md | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/packages/components/src/components/segment/readme.md b/packages/components/src/components/segment/readme.md index bc5a198334..249d97f3ad 100644 --- a/packages/components/src/components/segment/readme.md +++ b/packages/components/src/components/segment/readme.md @@ -7,24 +7,24 @@ ## Properties -| Property | Attribute | Description | Type | Default | -| ---------------------------- | ------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------ | ---------------------------------- | -------------- | -| `adjacentSiblings` | `adjacent-siblings` | | `"left" \| "leftright" \| "right"` | `undefined` | -| `ariaDescriptionTranslation` | `aria-description-translation` | a11y text for getting meaningful value. `$buttonNumber` and `$selected` are template variables and will be replaces by their corresponding properties. | `string` | `'$selected'` | -| `ariaLabelSegment` | `aria-label-segment` | (optional) aria-label attribute needed for icon-only segments | `string` | `undefined` | -| `ariaLangDeselected` | `aria-lang-deselected` | (optional) translation of 'deselected | `string` | `'deselected'` | -| `ariaLangSelected` | `aria-lang-selected` | (optional) translation of 'selected | `string` | `'selected'` | -| `disabled` | `disabled` | (optional) If `true`, the segment is disabled | `boolean` | `false` | -| `hasIcon` | `has-icon` | (optional) position within group | `boolean` | `undefined` | -| `iconOnly` | `icon-only` | (optional) position within group | `boolean` | `undefined` | -| `position` | `position` | (optional) position within group | `number` | `undefined` | -| `segmentId` | `segment-id` | (optional) segment's id | `string` | `undefined` | -| `selected` | `selected` | (optional) If `true`, the segment is selected | `boolean` | `false` | -| `selectedIndex` | `selected-index` | (optional) the index of the currently selected segment in the segmented-button | `string` | `undefined` | -| `size` | `size` | (optional) The size of the segment | `"large" \| "medium" \| "small"` | `'small'` | -| `styles` | `styles` | (optional) Injected CSS styles | `string` | `undefined` | -| `textOnly` | `text-only` | (optional) position within group | `boolean` | `undefined` | -| `width` | `width` | (optional) Segment width set to ensure that all segments have the same width | `string` | `undefined` | +| Property | Attribute | Description | Type | Default | +| ---------------------------- | ------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------ | ---------------------------------- | ------------------ | +| `adjacentSiblings` | `adjacent-siblings` | | `"left" \| "leftright" \| "right"` | `undefined` | +| `ariaDescriptionTranslation` | `aria-description-translation` | a11y text for getting meaningful value. `$buttonNumber` and `$selected` are template variables and will be replaces by their corresponding properties. | `string` | `'$selected'` | +| `ariaLabelSegment` | `aria-label-segment` | (optional) aria-label attribute needed for icon-only segments | `string` | `undefined` | +| `ariaLangDeselected` | `aria-lang-deselected` | (optional) translation of 'deselected | `string` | `'deselected'` | +| `ariaLangSelected` | `aria-lang-selected` | (optional) translation of 'selected | `string` | `'selected'` | +| `disabled` | `disabled` | (optional) If `true`, the segment is disabled | `boolean` | `false` | +| `hasIcon` | `has-icon` | (optional) position within group | `boolean` | `undefined` | +| `iconOnly` | `icon-only` | (optional) position within group | `boolean` | `undefined` | +| `position` | `position` | (optional) position within group | `number` | `undefined` | +| `segmentId` | `segment-id` | (optional) segment's id | `string` | `'segment-' + i++` | +| `selected` | `selected` | (optional) If `true`, the segment is selected | `boolean` | `false` | +| `selectedIndex` | `selected-index` | (optional) the index of the currently selected segment in the segmented-button | `string` | `undefined` | +| `size` | `size` | (optional) The size of the segment | `"large" \| "medium" \| "small"` | `'small'` | +| `styles` | `styles` | (optional) Injected CSS styles | `string` | `undefined` | +| `textOnly` | `text-only` | (optional) position within group | `boolean` | `undefined` | +| `width` | `width` | (optional) Segment width set to ensure that all segments have the same width | `string` | `undefined` | ## Events From 42ae0a659d7103953fe42fd1f91e4b1e2884fda1 Mon Sep 17 00:00:00 2001 From: Amir Ashkan Baghdoust Date: Thu, 17 Oct 2024 14:30:44 +0200 Subject: [PATCH 4/5] refactor(segment): add missing lifecycle hook and fix helper text display --- .../src/components/segment/segment.tsx | 4 +- .../segmented-button/segmented-button.tsx | 9 ++++- .../components/src/html/segment-button.html | 38 +++++++++++++++++-- 3 files changed, 45 insertions(+), 6 deletions(-) diff --git a/packages/components/src/components/segment/segment.tsx b/packages/components/src/components/segment/segment.tsx index d6aed90308..67b45ffa82 100644 --- a/packages/components/src/components/segment/segment.tsx +++ b/packages/components/src/components/segment/segment.tsx @@ -84,7 +84,9 @@ export class Segment { async setFocus() { this.focusableElement.focus(); } - + componentWillLoad() { + this.handleIcon(); + } componentWillUpdate() { this.handleIcon(); } diff --git a/packages/components/src/components/segmented-button/segmented-button.tsx b/packages/components/src/components/segmented-button/segmented-button.tsx index bc104f9523..e32fbc09ef 100644 --- a/packages/components/src/components/segmented-button/segmented-button.tsx +++ b/packages/components/src/components/segmented-button/segmented-button.tsx @@ -136,6 +136,7 @@ export class SegmentedButton { }); this.setState(tempState); this.selectedIndex = this.getSelectedIndex(); + this.showHelperText = this.shouldShowHelperText(); } componentDidLoad() { const longestButtonWidth = this.getLongestButtonWidth(); @@ -153,13 +154,17 @@ export class SegmentedButton { componentWillUpdate() { this.selectedIndex = this.getSelectedIndex(); - this.showHelperText = false; + this.showHelperText = this.shouldShowHelperText(); + } + shouldShowHelperText() { + let showHelperText = false; if ( this.invalid && this.status.filter((e) => e.selected === true).length <= 0 ) { - this.showHelperText = true; + showHelperText = true; } + return showHelperText; } getSelectedIndex() { diff --git a/packages/components/src/html/segment-button.html b/packages/components/src/html/segment-button.html index bf89b5ba49..771e502b3e 100644 --- a/packages/components/src/html/segment-button.html +++ b/packages/components/src/html/segment-button.html @@ -6,7 +6,7 @@ name="viewport" content="width=device-width, initial-scale=1.0, minimum-scale=1.0, maximum-scale=5.0" /> - Loading spinners + Segment Buttons @@ -15,6 +15,7 @@ section { padding: 1rem; } + .bg-black { background: black; color: white; @@ -32,17 +33,48 @@ Huawei - +
Apple One+ Samsung Huawei - +
Label Label +
+ + + + + Apple + + + + + One+ + + + + + Samsung + + + + + Huawei + + +
+ + + Apple + One+ + Samsung + Huawei + From 58aa9724d1501b8dfe6d4b27e20652b9ea7a9698 Mon Sep 17 00:00:00 2001 From: Amir Ashkan Baghdoust Date: Thu, 17 Oct 2024 15:00:08 +0200 Subject: [PATCH 5/5] refactor(segment): add missing span tag for label in segment.spec.ts.snap --- .../src/components/segment/__snapshots__/segment.spec.ts.snap | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/components/src/components/segment/__snapshots__/segment.spec.ts.snap b/packages/components/src/components/segment/__snapshots__/segment.spec.ts.snap index bf96880a50..1c95275796 100644 --- a/packages/components/src/components/segment/__snapshots__/segment.spec.ts.snap +++ b/packages/components/src/components/segment/__snapshots__/segment.spec.ts.snap @@ -15,6 +15,8 @@ exports[`Segment should match standard snapshot 1`] = `
- Label + + Label + `;