Skip to content

Comments

fix(segmented button): Scale Segmented Button doesn't respect controlled selected state (#2166)#2384

Merged
amir-ba merged 7 commits intotelekom:mainfrom
glebbo-dev:fix/#2166-segmented-button-doesnt-respect-controlled-selected-state
Mar 5, 2025
Merged

fix(segmented button): Scale Segmented Button doesn't respect controlled selected state (#2166)#2384
amir-ba merged 7 commits intotelekom:mainfrom
glebbo-dev:fix/#2166-segmented-button-doesnt-respect-controlled-selected-state

Conversation

@glebbo-dev
Copy link
Contributor

@glebbo-dev glebbo-dev commented Feb 5, 2025

Fixes #2166
Reason for this issue was that segmented button has never known when selected segment has been changed. Therefore I had to send an event when segment was selected outside the scale component and react on this event in segment button.

@netlify
Copy link

netlify bot commented Feb 5, 2025

Deploy Preview for marvelous-moxie-a6e2fe failed.

Name Link
🔨 Latest commit b596cd8
🔍 Latest deploy log https://app.netlify.com/sites/marvelous-moxie-a6e2fe/deploys/67c17a302027ef000863e183

@amir-ba
Copy link
Collaborator

amir-ba commented Feb 5, 2025

also run the lint and formatter npm command before you push your changes , otherwise the build pipeline will fail. Currently the preview deployment pipeline has some issues , but the build pipeline should work if you fix the linter and prettier issues.

Comment on lines 133 to 134
selected:
segment.hasAttribute('selected') && segment.selected ? true : false,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should keep this as it was, since any of hasAttribute and selected could be true. With HTML, they might look the same, but they are considered different. If the user sets the selected in the HTML tag, it could be parsed to any of this based on the framework it is being used.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn´t get your point. hasAttribute returns only if the attribute exists or not https://developer.mozilla.org/en-US/docs/Web/API/Element/hasAttribute . If we would let it as it is, this means every segment should be selected which has the attribute selected. It doesn´t matter if the selected is false or true. This means the initial state will be ignored.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point is that segment.hasAttribute('selected' and segment.selected are not always aligned. It could be that the DOM element is directly changed with 'setAttributeor the DOM element object was changed with.selected` not always these two are the sam e

@amir-ba
Copy link
Collaborator

amir-ba commented Mar 5, 2025

LGTM. tnx for the great job

@amir-ba amir-ba merged commit 39ad8ab into telekom:main Mar 5, 2025
8 of 12 checks passed
@glebbo-dev glebbo-dev deleted the fix/#2166-segmented-button-doesnt-respect-controlled-selected-state branch March 6, 2025 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Scale Segmented Button doesn't respect controlled selected state

2 participants