Skip to content

Commit 9d7a9db

Browse files
obstmiMichael Obst
andauthored
fix:#2400 infinite recursion in scale segmented button (#2421)
* fix: prevent event loop by distinguishing user and programmatic selection in scaleClick event * fix: format code with prettier * fix: fix linter errors * fix: some temporary test code * fix: simplified and prevent double emits of scale-click-events * fix: add toggle test to html page for changing the selected state * fix: add documentation * fix: prepare for pr --------- Co-authored-by: Michael Obst <michael.obst@telekom.de>
1 parent aedf22a commit 9d7a9db

File tree

6 files changed

+79
-14
lines changed

6 files changed

+79
-14
lines changed

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,10 @@
2929

3030
## Events
3131

32-
| Event | Description | Type |
33-
| ------------- | -------------------------------------------------------------------------------------------------- | ------------------------------------------------- |
34-
| `scale-click` | Emitted when button is clicked | `CustomEvent<{ id: string; selected: boolean; }>` |
35-
| `scaleClick` | <span style="color:red">**[DEPRECATED]**</span> in v3 in favor of kebab-case event names<br/><br/> | `CustomEvent<{ id: string; selected: boolean; }>` |
32+
| Event | Description | Type |
33+
| ------------- | -------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------- |
34+
| `scale-click` | Emitted when button is clicked | `CustomEvent<{ id: string; selected: boolean; userInteraction?: boolean; }>` |
35+
| `scaleClick` | <span style="color:red">**[DEPRECATED]**</span> in v3 in favor of kebab-case event names<br/><br/> | `CustomEvent<{ id: string; selected: boolean; userInteraction?: boolean; }>` |
3636

3737

3838
## Methods

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,21 +72,26 @@ export class Segment {
7272
@Event({ eventName: 'scale-click' }) scaleClick!: EventEmitter<{
7373
id: string;
7474
selected: boolean;
75+
userInteraction?: boolean;
7576
}>;
7677
/** @deprecated in v3 in favor of kebab-case event names */
7778
@Event({ eventName: 'scaleClick' }) scaleClickLegacy!: EventEmitter<{
7879
id: string;
7980
selected: boolean;
81+
userInteraction?: boolean;
8082
}>;
8183

8284
private focusableElement: HTMLElement;
85+
private userInteraction = false; // 'false' indicates that this event is triggered by internal state change
8386

8487
@Watch('selected')
8588
selectionChanged() {
8689
emitEvent(this, 'scaleClick', {
8790
id: this.segmentId,
8891
selected: this.selected,
92+
userInteraction: this.userInteraction,
8993
});
94+
this.userInteraction = false;
9095
}
9196

9297
@Method()
@@ -163,6 +168,7 @@ export class Segment {
163168
return;
164169
}
165170
event.preventDefault();
171+
this.userInteraction = true;
166172
this.selected = !this.selected;
167173
};
168174

packages/components/src/components/segmented-button/readme.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,10 @@
2424

2525
## Events
2626

27-
| Event | Description | Type |
28-
| -------------- | -------------------------------------------------------------------------------------------------- | ------------------ |
29-
| `scale-change` | Emitted when button is clicked | `CustomEvent<any>` |
30-
| `scaleChange` | <span style="color:red">**[DEPRECATED]**</span> in v3 in favor of kebab-case event names<br/><br/> | `CustomEvent<any>` |
27+
| Event | Description | Type |
28+
| -------------- | ----------------------------------------------------------------------------------------------------------------------------------- | ------------------ |
29+
| `scale-change` | Emitted when button is clicked. Not emitted in case of programmatic state changes (e.g. the `selected` state is set by the skript). | `CustomEvent<any>` |
30+
| `scaleChange` | <span style="color:red">**[DEPRECATED]**</span> in v3 in favor of kebab-case event names<br/><br/> | `CustomEvent<any>` |
3131

3232

3333
## Dependencies

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

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,15 +67,22 @@ export class SegmentedButton {
6767
ariaLabelTranslation = `segment button with $slottedSegments`;
6868
@Prop({ mutable: true })
6969
longestButtonWidth: string;
70-
/** Emitted when button is clicked */
70+
/** Emitted when button is clicked. Not emitted in case of programmatic state changes (e.g. the `selected` state is set by the skript). */
7171
@Event({ eventName: 'scale-change' }) scaleChange: EventEmitter;
7272
/** @deprecated in v3 in favor of kebab-case event names */
7373
@Event({ eventName: 'scaleChange' }) scaleChangeLegacy: EventEmitter;
7474

7575
container: HTMLElement;
7676
showHelperText = false;
7777
@Listen('scaleClick')
78-
scaleClickHandler(ev: { detail: { id: string; selected: boolean } }) {
78+
scaleClickHandler(
79+
ev: CustomEvent<{
80+
id: string;
81+
selected: boolean;
82+
userInteraction?: boolean;
83+
}>
84+
) {
85+
const { userInteraction = true } = ev.detail; // set default to true, which leads to emit the scaleChange-event finally
7986
let tempState = this.getAllSegments().map((segment) => {
8087
return {
8188
id: segment.segmentId,
@@ -92,9 +99,9 @@ export class SegmentedButton {
9299
ev.detail.id === obj.id ? ev.detail : { ...obj, selected: false }
93100
);
94101
}
95-
this.setState(tempState, ev.detail.selected);
102+
this.setState(tempState, userInteraction && ev.detail.selected);
96103
} else {
97-
this.setState(tempState);
104+
this.setState(tempState, userInteraction);
98105
}
99106
}
100107

packages/components/src/html/segment-button.html

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,12 +69,57 @@
6969
</scale-segment>
7070
</scale-segmented-button>
7171
<div style="height: 20px"></div>
72-
7372
<scale-segmented-button multi-select>
7473
<scale-segment selected>Apple</scale-segment>
7574
<scale-segment selected>One+</scale-segment>
7675
<scale-segment>Samsung</scale-segment>
7776
<scale-segment>Huawei</scale-segment>
7877
</scale-segmented-button>
78+
<div style="height: 20px"></div>
79+
<hr />
80+
<section>
81+
<scale-button id="toggle-button"
82+
>Change selected state of the segments</scale-button
83+
>
84+
<div style="height: 10px"></div>
85+
<scale-segmented-button id="segmented-button-toggle">
86+
<scale-segment id="segment-alpha">Option A</scale-segment>
87+
<scale-segment id="segment-beta" selected>Option B</scale-segment>
88+
</scale-segmented-button>
89+
<p>
90+
<b>state: <span id="state-display">b</span></b>
91+
</p>
92+
</section>
93+
<!-- functionality for changing the selected-state of the segments -->
94+
<script>
95+
let selectedOption = 'b';
96+
const toggleButton = document.getElementById('toggle-button');
97+
const segmentedButtonToggle = document.getElementById(
98+
'segmented-button-toggle'
99+
);
100+
const segmentAlpha = document.getElementById('segment-alpha');
101+
const segmentBeta = document.getElementById('segment-beta');
102+
const stateDisplay = document.getElementById('state-display');
103+
104+
function updateState() {
105+
segmentAlpha.selected = selectedOption === 'a';
106+
segmentBeta.selected = selectedOption === 'b';
107+
}
108+
109+
function onClick() {
110+
selectedOption = selectedOption === 'a' ? 'b' : 'a';
111+
updateState();
112+
stateDisplay.textContent = selectedOption;
113+
}
114+
115+
function onChange(event) {
116+
selectedOption = selectedOption === 'a' ? 'b' : 'a';
117+
updateState();
118+
stateDisplay.textContent = selectedOption;
119+
}
120+
121+
toggleButton.addEventListener('click', onClick);
122+
segmentedButtonToggle.addEventListener('scale-change', onChange);
123+
</script>
79124
</body>
80125
</html>

packages/storybook-vue/stories/components/segmented-button/SegmentedButton.stories.mdx

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,14 @@ import ScaleSegmentedButton from './ScaleSegmentedButton.vue';
1010
},
1111
ariaLabelTranslation: {
1212
description: "(optional) aria-label attribute needed for icon-only buttons"
13-
}
13+
},
14+
scaleChange: {
15+
description: '@deprecated in v3 in favor of kebab-case event names.',
16+
},
17+
'scale-change': {
18+
description:
19+
'Emitted when button is clicked. Not emitted in case of programmatic state changes (e.g. the `selected` state is set by the skript).',
20+
},
1421
}}
1522
/>
1623

0 commit comments

Comments
 (0)