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.

Sign up for GitHub

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

Jump to bottom

RadioSet should not query the DOM to get its state. #2203

Closed
willmcgugan opened this issue Apr 3, 2023 · 2 comments · Fixed by #2205
Closed

RadioSet should not query the DOM to get its state. #2203

willmcgugan opened this issue Apr 3, 2023 · 2 comments · Fixed by #2205
Assignees
@davep
Labels
enhancement New feature or request Task

Comments

@willmcgugan
Copy link
Collaborator

With the current implementation, there is a brief window where there can be two radio buttons checked. This can lead to the pressed_button property returning None.

We should not use the DOM in this instance. Better to keep track of the pressed button in an attribute.
@davep davep added enhancement New feature or request Task labels Apr 3, 2023
@davep
Copy link
Contributor

davep commented Apr 3, 2023

See #2202

@davep davep self-assigned this Apr 3, 2023
davep added a commit to davep/textual that referenced this issue Apr 3, 2023
@davep
Rework RadioSet so that it doesn't lean so heavily on the DOM
a8beacc 
Stemming from Textualize#2202 and implementing the solution decided in Textualize#2203. Pretty
much this change removes all the state change/handling being done on the DOM
and keeps the state internally.
@davep davep mentioned this issue Apr 3, 2023
Merged
@davep davep linked a pull request Apr 3, 2023 that will close this issue
Rework RadioSet so it no longer leans on the DOM for state #2205
Merged
@davep davep changed the title RadioSet should not query the DOM to get it's state. RadioSet should not query the DOM to get its state. Apr 3, 2023
@github-actions
Copy link

github-actions bot commented Apr 4, 2023

Don't forget to star the repository!

Follow @textualizeio for Textual updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@davep davep

Labels
enhancement New feature or request Task
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Rework RadioSet so it no longer leans on the DOM for state davep/textual
2 participants
@davep @willmcgugan