Skip to content

Commit 39ad8ab

Browse files
glebbo-devglebbo-dev
authored
fix(segmented button): Scale Segmented Button doesn't respect controlled selected state (#2166) (#2384)
* fix(segmented button): added event handler to change selected index of segment item (#2166) * fix(segmented-button): refactored event handler and adjusted code by merge request comments * fix(segmented-button): adjusted code from comments and fixed snap * fix(segmented-button): set selected index to -2 * fix(segmented-button): set selected index to 0 as first segment should be selected * fix(segmented-button): removed unnecessary getSelectedIndex * fix(segmented-button): adjusted segmented button to scale attributes and updated snapshot
1 parent 525e1c4 commit 39ad8ab

File tree

5 files changed

+49
-56
lines changed

5 files changed

+49
-56
lines changed

‎packages/components/src/components/segment/readme.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
| `position` | `position` | (optional) position within group | `number` | `undefined` |
2121
| `segmentId` | `segment-id` | (optional) segment's id | `string` | `'segment-' + i++` |
2222
| `selected` | `selected` | (optional) If `true`, the segment is selected | `boolean` | `false` |
23-
| `selectedIndex` | `selected-index` | (optional) the index of the currently selected segment in the segmented-button | `string` | `undefined` |
23+
| `selectedIndex` | `selected-index` | (optional) the index of the currently selected segment in the segmented-button | `number` | `undefined` |
2424
| `size` | `size` | (optional) The size of the segment | `"large" \| "medium" \| "small"` | `'small'` |
2525
| `styles` | `styles` | (optional) Injected CSS styles | `string` | `undefined` |
2626
| `textOnly` | `text-only` | (optional) position within group | `boolean` | `undefined` |

‎packages/components/src/components/segment/segment.tsx‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
Event,
1919
EventEmitter,
2020
Method,
21+
Watch,
2122
} from '@stencil/core';
2223
import classNames from 'classnames';
2324
import { emitEvent } from '../../utils/utils';
@@ -65,7 +66,7 @@ export class Segment {
6566
/** (optional) position within group */
6667
@Prop({ mutable: true }) iconOnly?: boolean;
6768
/** (optional) the index of the currently selected segment in the segmented-button */
68-
@Prop({ mutable: true }) selectedIndex?: string;
69+
@Prop({ mutable: true }) selectedIndex?: number;
6970

7071
/** Emitted when button is clicked */
7172
@Event({ eventName: 'scale-click' }) scaleClick!: EventEmitter<{
@@ -80,6 +81,14 @@ export class Segment {
8081

8182
private focusableElement: HTMLElement;
8283

84+
@Watch('selected')
85+
selectionChanged() {
86+
emitEvent(this, 'scaleClick', {
87+
id: this.segmentId,
88+
selected: this.selected,
89+
});
90+
}
91+
8392
@Method()
8493
async setFocus() {
8594
this.focusableElement.focus();
@@ -150,15 +159,11 @@ export class Segment {
150159
}
151160

152161
handleClick = (event: MouseEvent) => {
153-
if (parseInt(this.selectedIndex, 10) + 1 === this.position) {
162+
if (this.selectedIndex === this.position) {
154163
return;
155164
}
156165
event.preventDefault();
157166
this.selected = !this.selected;
158-
emitEvent(this, 'scaleClick', {
159-
id: this.segmentId,
160-
selected: this.selected,
161-
});
162167
};
163168

164169
render() {

‎packages/components/src/components/segmented-button/__snapshots__/segmented-button.spec.ts.snap‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,10 @@ exports[`SegmentedButton should match selected button snapshot 1`] = `
77
<slot></slot>
88
</div>
99
</mock:shadow-root>
10-
<scale-segment adjacent-siblings="right" aria-description-translation="$position $selected" position="1" selected="true" selected-index="-1" size="small">
10+
<scale-segment selected="">
1111
Label
1212
</scale-segment>
13-
<scale-segment adjacent-siblings="left" aria-description-translation="$position $selected" position="2" selected="true" selected-index="-1" size="small">
13+
<scale-segment>
1414
Label
1515
</scale-segment>
1616
</scale-segmented-button>
@@ -23,17 +23,17 @@ exports[`SegmentedButton should match standard snapshot 1`] = `
2323
<slot></slot>
2424
</div>
2525
</mock:shadow-root>
26-
<scale-segment adjacent-siblings="" aria-description-translation="$position $selected" position="1" selected="false" selected-index="-1" size="small">
26+
<scale-segment>
2727
Label
2828
</scale-segment>
29-
<scale-segment adjacent-siblings="" aria-description-translation="$position $selected" position="2" selected="false" selected-index="-1" size="small">
29+
<scale-segment>
3030
Label
3131
</scale-segment>
32-
<scale-segment adjacent-siblings="" aria-description-translation="$position $selected" position="3" selected="false" selected-index="-1" size="small">
32+
<scale-segment>
3333
Label
3434
</scale-segment>
35-
<scale-segment adjacent-siblings="" aria-description-translation="$position $selected" position="4" selected="false" selected-index="-1" size="small">
35+
<scale-segment>
3636
Label
3737
</scale-segment>
3838
</scale-segmented-button>
39-
`;
39+
`;

‎packages/components/src/components/segmented-button/segmented-button.spec.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ describe('SegmentedButton', () => {
4343
html: `
4444
<scale-segmented-button>
4545
<scale-segment selected>Label</scale-segment>
46-
<scale-segment selected>Label</scale-segment>
46+
<scale-segment>Label</scale-segment>
4747
</scale-segmented-button>`,
4848
});
4949
expect(page.root).toMatchSnapshot();

‎packages/components/src/components/segmented-button/segmented-button.tsx‎

Lines changed: 29 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import {
1515
h,
1616
Host,
1717
Element,
18-
State,
1918
Listen,
2019
Event,
2120
EventEmitter,
@@ -45,8 +44,6 @@ export class SegmentedButton {
4544
slottedSegments = 0;
4645

4746
@Element() hostElement: HTMLElement;
48-
/** state */
49-
@State() status: SegmentStatus[] = [];
5047
/** (optional) The size of the button */
5148
@Prop() size?: 'small' | 'medium' | 'large' = 'small';
5249
/** (optional) Allow more than one button to be selected */
@@ -79,24 +76,26 @@ export class SegmentedButton {
7976
showHelperText = false;
8077
@Listen('scaleClick')
8178
scaleClickHandler(ev: { detail: { id: string; selected: boolean } }) {
82-
let tempState: SegmentStatus[];
79+
let tempState = this.getAllSegments().map((segment) => {
80+
return {
81+
id: segment.segmentId,
82+
selected: segment.selected,
83+
};
84+
});
8385
if (!this.multiSelect) {
8486
if (!ev.detail.selected) {
85-
tempState = this.status.map((obj) =>
87+
tempState = tempState.map((obj) =>
8688
ev.detail.id === obj.id ? ev.detail : { ...obj }
8789
);
88-
/* clicked button has now selected state */
8990
} else {
90-
tempState = this.status.map((obj) =>
91+
tempState = tempState.map((obj) =>
9192
ev.detail.id === obj.id ? ev.detail : { ...obj, selected: false }
9293
);
9394
}
95+
this.setState(tempState, ev.detail.selected);
9496
} else {
95-
tempState = this.status.map((obj) =>
96-
ev.detail.id === obj.id ? ev.detail : { ...obj }
97-
);
97+
this.setState(tempState);
9898
}
99-
this.setState(tempState);
10099
}
101100

102101
@Watch('disabled')
@@ -111,10 +110,10 @@ export class SegmentedButton {
111110
*/
112111
propagatePropsToChildren() {
113112
this.getAllSegments().forEach((segment) => {
114-
segment.setAttribute('size', this.size);
115-
segment.setAttribute('selected-index', this.selectedIndex.toString());
113+
segment.size = this.size;
114+
segment.selectedIndex = this.selectedIndex;
116115
if (this.disabled) {
117-
segment.setAttribute('disabled', true && 'disabled');
116+
segment.disabled = true;
118117
}
119118
});
120119
}
@@ -125,17 +124,13 @@ export class SegmentedButton {
125124
this.slottedSegments = segments.length;
126125
segments.forEach((segment, i) => {
127126
tempState.push({
128-
id: segment.getAttribute('segment-id') || segment.segmentId,
129-
selected: segment.hasAttribute('selected') || segment.selected,
127+
id: segment.segmentId,
128+
selected: segment.selected,
130129
});
131-
segment.setAttribute('position', `${i + 1}`);
132-
segment.setAttribute(
133-
'aria-description-translation',
134-
'$position $selected'
135-
);
130+
segment.position = i;
131+
segment.ariaDescriptionTranslation = '$position $selected';
136132
});
137-
this.setState(tempState);
138-
this.selectedIndex = this.getSelectedIndex();
133+
this.setState(tempState, false);
139134
this.showHelperText = this.shouldShowHelperText();
140135
}
141136
componentDidLoad() {
@@ -153,15 +148,11 @@ export class SegmentedButton {
153148
}
154149

155150
componentWillUpdate() {
156-
this.selectedIndex = this.getSelectedIndex();
157151
this.showHelperText = this.shouldShowHelperText();
158152
}
159153
shouldShowHelperText() {
160154
let showHelperText = false;
161-
if (
162-
this.invalid &&
163-
this.status.filter((e) => e.selected === true).length <= 0
164-
) {
155+
if (this.invalid && this.selectedIndex < 0) {
165156
showHelperText = true;
166157
}
167158
return showHelperText;
@@ -176,12 +167,13 @@ export class SegmentedButton {
176167
const selectedIndex = allSegments.findIndex(
177168
(el: HTMLScaleSegmentElement) => el.selected === true
178169
);
179-
return selectedIndex;
170+
// we need to return -2 if no segment is selected
171+
return selectedIndex >= 0 ? selectedIndex : -2;
180172
}
181173
}
182174

183175
getAdjacentSiblings = (tempState, i) => {
184-
let adjacentSiblings = '';
176+
let adjacentSiblings = null;
185177
if (i !== 0 && tempState[i].selected && tempState[i - 1].selected) {
186178
adjacentSiblings = 'left';
187179
}
@@ -226,22 +218,18 @@ export class SegmentedButton {
226218
return tempWidth;
227219
}
228220

229-
setState(tempState: SegmentStatus[]) {
221+
setState(tempState: SegmentStatus[], handleEvent: boolean = true) {
230222
const segments = Array.from(
231223
this.hostElement.querySelectorAll('scale-segment')
232224
);
233225
segments.forEach((segment, i) => {
234-
segment.setAttribute(
235-
'adjacent-siblings',
236-
this.getAdjacentSiblings(tempState, i)
237-
);
238-
segment.setAttribute(
239-
'selected',
240-
tempState[i].selected ? 'true' : 'false'
241-
);
226+
segment.adjacentSiblings = this.getAdjacentSiblings(tempState, i);
227+
segment.selected = tempState[i].selected;
242228
});
243-
this.status = tempState;
244-
emitEvent(this, 'scaleChange', this.status);
229+
this.selectedIndex = this.getSelectedIndex();
230+
if (handleEvent) {
231+
emitEvent(this, 'scaleChange', { segments });
232+
}
245233
}
246234

247235
getAllSegments() {

0 commit comments

Comments
 (0)