From 70f030aea1240c93d3a8974778f925a8a7df7d7a Mon Sep 17 00:00:00 2001 From: glebbo <75304736+glebbo-dev@users.noreply.github.com> Date: Wed, 5 Feb 2025 10:32:39 +0100 Subject: [PATCH 1/7] fix(segmented button): added event handler to change selected index of segment item (#2166) --- packages/components/src/components/segment/segment.tsx | 8 ++++++++ .../src/components/segmented-button/segmented-button.tsx | 9 +++++++++ 2 files changed, 17 insertions(+) diff --git a/packages/components/src/components/segment/segment.tsx b/packages/components/src/components/segment/segment.tsx index 67b45ffa82..dab32df393 100644 --- a/packages/components/src/components/segment/segment.tsx +++ b/packages/components/src/components/segment/segment.tsx @@ -18,6 +18,7 @@ import { Event, EventEmitter, Method, + Watch, } from '@stencil/core'; import classNames from 'classnames'; import { emitEvent } from '../../utils/utils'; @@ -72,6 +73,8 @@ export class Segment { id: string; selected: boolean; }>; + /** Emitted when selection has been changed. */ + @Event({ eventName: 'scaleSelectionChanged' }) scaleSelectionChanged!: EventEmitter; /** @deprecated in v3 in favor of kebab-case event names */ @Event({ eventName: 'scaleClick' }) scaleClickLegacy!: EventEmitter<{ id: string; @@ -80,6 +83,11 @@ export class Segment { private focusableElement: HTMLElement; + @Watch('selected') + selectionChanged() { + emitEvent(this, 'scaleSelectionChanged'); + } + @Method() async setFocus() { this.focusableElement.focus(); diff --git a/packages/components/src/components/segmented-button/segmented-button.tsx b/packages/components/src/components/segmented-button/segmented-button.tsx index e32fbc09ef..9e98d129d2 100644 --- a/packages/components/src/components/segmented-button/segmented-button.tsx +++ b/packages/components/src/components/segmented-button/segmented-button.tsx @@ -99,6 +99,15 @@ export class SegmentedButton { this.setState(tempState); } + @Listen('scaleSelectionChanged') + selectionChangedHandler() { + this.selectedIndex = this.getSelectedIndex(); + if (typeof(this.selectedIndex) !== 'number') { return } + this.getAllSegments().forEach((segment) => { + segment.setAttribute('selected-index', this.selectedIndex.toString()); + }) + } + @Watch('disabled') @Watch('size') @Watch('selectedIndex') From 52728a9ca57649380e3c5a641c9785078cd6a759 Mon Sep 17 00:00:00 2001 From: glebbo <75304736+glebbo-dev@users.noreply.github.com> Date: Mon, 17 Feb 2025 09:44:59 +0100 Subject: [PATCH 2/7] fix(segmented-button): refactored event handler and adjusted code by merge request comments --- .../src/components/segment/readme.md | 2 +- .../src/components/segment/segment.tsx | 18 ++++---- .../segmented-button/segmented-button.tsx | 45 +++++++++---------- 3 files changed, 32 insertions(+), 33 deletions(-) diff --git a/packages/components/src/components/segment/readme.md b/packages/components/src/components/segment/readme.md index 249d97f3ad..089e5cd0ec 100644 --- a/packages/components/src/components/segment/readme.md +++ b/packages/components/src/components/segment/readme.md @@ -20,7 +20,7 @@ | `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` | +| `selectedIndex` | `selected-index` | (optional) the index of the currently selected segment in the segmented-button | `number` | `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` | diff --git a/packages/components/src/components/segment/segment.tsx b/packages/components/src/components/segment/segment.tsx index dab32df393..c57d21dc14 100644 --- a/packages/components/src/components/segment/segment.tsx +++ b/packages/components/src/components/segment/segment.tsx @@ -66,15 +66,13 @@ export class Segment { /** (optional) position within group */ @Prop({ mutable: true }) iconOnly?: boolean; /** (optional) the index of the currently selected segment in the segmented-button */ - @Prop({ mutable: true }) selectedIndex?: string; + @Prop({ mutable: true }) selectedIndex?: number; /** Emitted when button is clicked */ @Event({ eventName: 'scale-click' }) scaleClick!: EventEmitter<{ id: string; selected: boolean; }>; - /** Emitted when selection has been changed. */ - @Event({ eventName: 'scaleSelectionChanged' }) scaleSelectionChanged!: EventEmitter; /** @deprecated in v3 in favor of kebab-case event names */ @Event({ eventName: 'scaleClick' }) scaleClickLegacy!: EventEmitter<{ id: string; @@ -85,7 +83,13 @@ export class Segment { @Watch('selected') selectionChanged() { - emitEvent(this, 'scaleSelectionChanged'); + if (this.selectedIndex !== -1 && !this.selected) { + return; + } + emitEvent(this, 'scaleClick', { + id: this.segmentId, + selected: this.selected, + }); } @Method() @@ -158,15 +162,11 @@ export class Segment { } handleClick = (event: MouseEvent) => { - if (parseInt(this.selectedIndex, 10) + 1 === this.position) { + if (this.selectedIndex === this.position) { return; } event.preventDefault(); this.selected = !this.selected; - emitEvent(this, 'scaleClick', { - id: this.segmentId, - selected: this.selected, - }); }; render() { diff --git a/packages/components/src/components/segmented-button/segmented-button.tsx b/packages/components/src/components/segmented-button/segmented-button.tsx index 9e98d129d2..83c53e75d8 100644 --- a/packages/components/src/components/segmented-button/segmented-button.tsx +++ b/packages/components/src/components/segmented-button/segmented-button.tsx @@ -79,35 +79,30 @@ export class SegmentedButton { showHelperText = false; @Listen('scaleClick') scaleClickHandler(ev: { detail: { id: string; selected: boolean } }) { - let tempState: SegmentStatus[]; + if (this.status.length === 0) { + return; + } + let tempState = this.getAllSegments().map((segment) => { + return { + id: segment.getAttribute('segment-id') || segment.segmentId, + selected: + segment.hasAttribute('selected') && segment.selected ? true : false, + }; + }); if (!this.multiSelect) { if (!ev.detail.selected) { - tempState = this.status.map((obj) => + tempState = tempState.map((obj) => ev.detail.id === obj.id ? ev.detail : { ...obj } ); - /* clicked button has now selected state */ } else { - tempState = this.status.map((obj) => + tempState = tempState.map((obj) => ev.detail.id === obj.id ? ev.detail : { ...obj, selected: false } ); } - } else { - tempState = this.status.map((obj) => - ev.detail.id === obj.id ? ev.detail : { ...obj } - ); } this.setState(tempState); } - @Listen('scaleSelectionChanged') - selectionChangedHandler() { - this.selectedIndex = this.getSelectedIndex(); - if (typeof(this.selectedIndex) !== 'number') { return } - this.getAllSegments().forEach((segment) => { - segment.setAttribute('selected-index', this.selectedIndex.toString()); - }) - } - @Watch('disabled') @Watch('size') @Watch('selectedIndex') @@ -135,15 +130,16 @@ export class SegmentedButton { segments.forEach((segment, i) => { tempState.push({ id: segment.getAttribute('segment-id') || segment.segmentId, - selected: segment.hasAttribute('selected') || segment.selected, + selected: + segment.hasAttribute('selected') && segment.selected ? true : false, }); - segment.setAttribute('position', `${i + 1}`); + segment.setAttribute('position', `${i}`); segment.setAttribute( 'aria-description-translation', '$position $selected' ); }); - this.setState(tempState); + this.setState(tempState, true); this.selectedIndex = this.getSelectedIndex(); this.showHelperText = this.shouldShowHelperText(); } @@ -185,7 +181,8 @@ export class SegmentedButton { const selectedIndex = allSegments.findIndex( (el: HTMLScaleSegmentElement) => el.selected === true ); - return selectedIndex; + // we need to return -2 if no segment is selected + return selectedIndex >= 0 ? selectedIndex : -2; } } @@ -235,7 +232,7 @@ export class SegmentedButton { return tempWidth; } - setState(tempState: SegmentStatus[]) { + setState(tempState: SegmentStatus[], isInitial: boolean = false) { const segments = Array.from( this.hostElement.querySelectorAll('scale-segment') ); @@ -250,7 +247,9 @@ export class SegmentedButton { ); }); this.status = tempState; - emitEvent(this, 'scaleChange', this.status); + if (!isInitial) { + emitEvent(this, 'scaleChange', { status: this.status, segments }); + } } getAllSegments() { From c4cfb011a27dfb778406f00d094d2ca6f2911bfa Mon Sep 17 00:00:00 2001 From: glebbo <75304736+glebbo-dev@users.noreply.github.com> Date: Mon, 24 Feb 2025 09:00:59 +0100 Subject: [PATCH 3/7] fix(segmented-button): adjusted code from comments and fixed snap --- .../__snapshots__/segmented-button.spec.ts.snap | 10 +++++----- .../segmented-button/segmented-button.tsx | 16 +++------------- 2 files changed, 8 insertions(+), 18 deletions(-) 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 82e5bc5170..76b20c3699 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 @@ -7,10 +7,10 @@ exports[`SegmentedButton should match selected button snapshot 1`] = ` - + Label - + Label @@ -23,6 +23,9 @@ exports[`SegmentedButton should match standard snapshot 1`] = ` + + Label + Label @@ -32,8 +35,5 @@ exports[`SegmentedButton should match standard snapshot 1`] = ` Label - - 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 83c53e75d8..e5212a89f9 100644 --- a/packages/components/src/components/segmented-button/segmented-button.tsx +++ b/packages/components/src/components/segmented-button/segmented-button.tsx @@ -15,7 +15,6 @@ import { h, Host, Element, - State, Listen, Event, EventEmitter, @@ -45,8 +44,6 @@ export class SegmentedButton { slottedSegments = 0; @Element() hostElement: HTMLElement; - /** state */ - @State() status: SegmentStatus[] = []; /** (optional) The size of the button */ @Prop() size?: 'small' | 'medium' | 'large' = 'small'; /** (optional) Allow more than one button to be selected */ @@ -79,9 +76,6 @@ export class SegmentedButton { showHelperText = false; @Listen('scaleClick') scaleClickHandler(ev: { detail: { id: string; selected: boolean } }) { - if (this.status.length === 0) { - return; - } let tempState = this.getAllSegments().map((segment) => { return { id: segment.getAttribute('segment-id') || segment.segmentId, @@ -158,15 +152,11 @@ export class SegmentedButton { } componentWillUpdate() { - this.selectedIndex = this.getSelectedIndex(); this.showHelperText = this.shouldShowHelperText(); } shouldShowHelperText() { let showHelperText = false; - if ( - this.invalid && - this.status.filter((e) => e.selected === true).length <= 0 - ) { + if (this.invalid && this.selectedIndex < 0) { showHelperText = true; } return showHelperText; @@ -246,9 +236,9 @@ export class SegmentedButton { tempState[i].selected ? 'true' : 'false' ); }); - this.status = tempState; + this.selectedIndex = this.getSelectedIndex(); if (!isInitial) { - emitEvent(this, 'scaleChange', { status: this.status, segments }); + emitEvent(this, 'scaleChange', { segments }); } } From e251d69941a47b6248b584278bbece692a37fd76 Mon Sep 17 00:00:00 2001 From: glebbo <75304736+glebbo-dev@users.noreply.github.com> Date: Mon, 24 Feb 2025 09:21:10 +0100 Subject: [PATCH 4/7] fix(segmented-button): set selected index to -2 --- .../__snapshots__/segmented-button.spec.ts.snap | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) 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 76b20c3699..f63cd2b1d7 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 @@ -7,10 +7,10 @@ exports[`SegmentedButton should match selected button snapshot 1`] = ` - + Label - + Label @@ -23,16 +23,16 @@ exports[`SegmentedButton should match standard snapshot 1`] = ` - + Label - + Label - + Label - + Label From 2959699a2cae4fff285bebf96c94304889fb4299 Mon Sep 17 00:00:00 2001 From: glebbo <75304736+glebbo-dev@users.noreply.github.com> Date: Mon, 24 Feb 2025 09:30:26 +0100 Subject: [PATCH 5/7] fix(segmented-button): set selected index to 0 as first segment should be selected --- .../__snapshots__/segmented-button.spec.ts.snap | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 f63cd2b1d7..206a9da4aa 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 @@ -7,10 +7,10 @@ exports[`SegmentedButton should match selected button snapshot 1`] = ` - + Label - + Label From 3e4c78017ac176e533bbdbbd0ce5f1d94c94f125 Mon Sep 17 00:00:00 2001 From: glebbo <75304736+glebbo-dev@users.noreply.github.com> Date: Mon, 24 Feb 2025 09:43:26 +0100 Subject: [PATCH 6/7] fix(segmented-button): removed unnecessary getSelectedIndex --- .../src/components/segmented-button/segmented-button.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/components/src/components/segmented-button/segmented-button.tsx b/packages/components/src/components/segmented-button/segmented-button.tsx index e5212a89f9..6e7949e7e3 100644 --- a/packages/components/src/components/segmented-button/segmented-button.tsx +++ b/packages/components/src/components/segmented-button/segmented-button.tsx @@ -134,7 +134,6 @@ export class SegmentedButton { ); }); this.setState(tempState, true); - this.selectedIndex = this.getSelectedIndex(); this.showHelperText = this.shouldShowHelperText(); } componentDidLoad() { From b596cd874cdd57b55314911c1dfe14b8c5d7948f Mon Sep 17 00:00:00 2001 From: glebbo <75304736+glebbo-dev@users.noreply.github.com> Date: Fri, 28 Feb 2025 09:55:55 +0100 Subject: [PATCH 7/7] fix(segmented-button): adjusted segmented button to scale attributes and updated snapshot --- .../src/components/segment/segment.tsx | 3 -- .../segmented-button.spec.ts.snap | 14 +++--- .../segmented-button/segmented-button.spec.ts | 2 +- .../segmented-button/segmented-button.tsx | 45 ++++++++----------- 4 files changed, 26 insertions(+), 38 deletions(-) diff --git a/packages/components/src/components/segment/segment.tsx b/packages/components/src/components/segment/segment.tsx index c57d21dc14..29ddffb507 100644 --- a/packages/components/src/components/segment/segment.tsx +++ b/packages/components/src/components/segment/segment.tsx @@ -83,9 +83,6 @@ export class Segment { @Watch('selected') selectionChanged() { - if (this.selectedIndex !== -1 && !this.selected) { - return; - } emitEvent(this, 'scaleClick', { id: this.segmentId, selected: this.selected, 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 206a9da4aa..f3cc15e81d 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 @@ -7,10 +7,10 @@ exports[`SegmentedButton should match selected button snapshot 1`] = ` - + Label - + Label @@ -23,17 +23,17 @@ exports[`SegmentedButton should match standard snapshot 1`] = ` - + Label - + Label - + Label - + Label -`; \ No newline at end of file +`; diff --git a/packages/components/src/components/segmented-button/segmented-button.spec.ts b/packages/components/src/components/segmented-button/segmented-button.spec.ts index aadc728f1e..51392c20e6 100644 --- a/packages/components/src/components/segmented-button/segmented-button.spec.ts +++ b/packages/components/src/components/segmented-button/segmented-button.spec.ts @@ -43,7 +43,7 @@ describe('SegmentedButton', () => { html: ` Label - Label + Label `, }); expect(page.root).toMatchSnapshot(); diff --git a/packages/components/src/components/segmented-button/segmented-button.tsx b/packages/components/src/components/segmented-button/segmented-button.tsx index 6e7949e7e3..11982f9805 100644 --- a/packages/components/src/components/segmented-button/segmented-button.tsx +++ b/packages/components/src/components/segmented-button/segmented-button.tsx @@ -78,9 +78,8 @@ export class SegmentedButton { scaleClickHandler(ev: { detail: { id: string; selected: boolean } }) { let tempState = this.getAllSegments().map((segment) => { return { - id: segment.getAttribute('segment-id') || segment.segmentId, - selected: - segment.hasAttribute('selected') && segment.selected ? true : false, + id: segment.segmentId, + selected: segment.selected, }; }); if (!this.multiSelect) { @@ -93,8 +92,10 @@ export class SegmentedButton { ev.detail.id === obj.id ? ev.detail : { ...obj, selected: false } ); } + this.setState(tempState, ev.detail.selected); + } else { + this.setState(tempState); } - this.setState(tempState); } @Watch('disabled') @@ -109,10 +110,10 @@ export class SegmentedButton { */ propagatePropsToChildren() { this.getAllSegments().forEach((segment) => { - segment.setAttribute('size', this.size); - segment.setAttribute('selected-index', this.selectedIndex.toString()); + segment.size = this.size; + segment.selectedIndex = this.selectedIndex; if (this.disabled) { - segment.setAttribute('disabled', true && 'disabled'); + segment.disabled = true; } }); } @@ -123,17 +124,13 @@ export class SegmentedButton { this.slottedSegments = segments.length; segments.forEach((segment, i) => { tempState.push({ - id: segment.getAttribute('segment-id') || segment.segmentId, - selected: - segment.hasAttribute('selected') && segment.selected ? true : false, + id: segment.segmentId, + selected: segment.selected, }); - segment.setAttribute('position', `${i}`); - segment.setAttribute( - 'aria-description-translation', - '$position $selected' - ); + segment.position = i; + segment.ariaDescriptionTranslation = '$position $selected'; }); - this.setState(tempState, true); + this.setState(tempState, false); this.showHelperText = this.shouldShowHelperText(); } componentDidLoad() { @@ -176,7 +173,7 @@ export class SegmentedButton { } getAdjacentSiblings = (tempState, i) => { - let adjacentSiblings = ''; + let adjacentSiblings = null; if (i !== 0 && tempState[i].selected && tempState[i - 1].selected) { adjacentSiblings = 'left'; } @@ -221,22 +218,16 @@ export class SegmentedButton { return tempWidth; } - setState(tempState: SegmentStatus[], isInitial: boolean = false) { + setState(tempState: SegmentStatus[], handleEvent: boolean = true) { const segments = Array.from( this.hostElement.querySelectorAll('scale-segment') ); segments.forEach((segment, i) => { - segment.setAttribute( - 'adjacent-siblings', - this.getAdjacentSiblings(tempState, i) - ); - segment.setAttribute( - 'selected', - tempState[i].selected ? 'true' : 'false' - ); + segment.adjacentSiblings = this.getAdjacentSiblings(tempState, i); + segment.selected = tempState[i].selected; }); this.selectedIndex = this.getSelectedIndex(); - if (!isInitial) { + if (handleEvent) { emitEvent(this, 'scaleChange', { segments }); } }