Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: "disappearing messages" dialog wrong value #4327

Merged
merged 2 commits into from
Nov 9, 2024

Conversation

WofWca
Copy link
Collaborator

@WofWca WofWca commented Nov 8, 2024

The bug was introduced in 0e0d0b8,
where we switched from the Blueprint's RadioGroup to our
custom RadioGroup, which had the bug where it would not
reflect the actual value if it was changed programmatically.
This commit fixes that bug.

The bug in Radio existed ever since its introduction.
The reason for using defaultChecked instead of checked
probably was the fact that React would complain that
"input doesn't have onChange listener set,
use defaultChecked instead".

I have ligtly checked that "disappearing messages"
is the only affected place.

@WofWca WofWca force-pushed the wofwca/fix-radio-group-value branch from bc9f5a0 to 5e61542 Compare November 8, 2024 14:49
@WofWca WofWca marked this pull request as ready for review November 8, 2024 14:50
@WofWca WofWca force-pushed the wofwca/fix-radio-group-value branch from 5e61542 to 743e453 Compare November 8, 2024 14:53
@WofWca WofWca added bug Something isn't working polish labels Nov 8, 2024
@WofWca WofWca marked this pull request as draft November 8, 2024 14:58
@WofWca
Copy link
Collaborator Author

WofWca commented Nov 8, 2024

Hold up, this prints warnings now

The bug was introduced in 0e0d0b8,
where we switched from the Blueprint's `RadioGroup` to our
custom `RadioGroup`, which had the bug where it would not
reflect the actual value if it was changed programmatically.
This commit fixes that bug.

The bug in `Radio` existed ever since its introduction.
The reason for using `defaultChecked` instead of `checked`
probably was the fact that React would complain that
"input doesn't have `onChange` listener set,
use `defaultChecked` instead".

I have ligtly checked that "disappearing messages"
is the only affected place.
@WofWca WofWca force-pushed the wofwca/fix-radio-group-value branch from 743e453 to a120989 Compare November 8, 2024 15:09
@WofWca WofWca assigned WofWca and unassigned WofWca Nov 8, 2024
@WofWca WofWca marked this pull request as ready for review November 8, 2024 15:10
Copy link
Member

@Simon-Laux Simon-Laux left a comment

Choose a reason for hiding this comment

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

code LGTM, didn't test

@WofWca WofWca merged commit b541b66 into main Nov 9, 2024
10 checks passed
@WofWca WofWca deleted the wofwca/fix-radio-group-value branch November 9, 2024 08:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working polish
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants