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

Implement RadioGroup::{row,column,for_axis} to allow layout customization #2157

Merged
merged 3 commits into from
Mar 28, 2022
Merged

Implement RadioGroup::{row,column,for_axis} to allow layout customization #2157

merged 3 commits into from
Mar 28, 2022

Conversation

twitchyliquid64
Copy link
Contributor

@twitchyliquid64 twitchyliquid64 commented Mar 25, 2022

Trivial change to allow the user to set the Axis on which Radio's are laid out. Breaking change in that RadioGroup::new is now RadioGroup::column.

Builder methods column(), row(), for_axis() are consistent with the Flex widget.

Copy link
Collaborator

@jneem jneem left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I'd be inclined to copy the Flex naming for consistency, even if it is a breaking change. (e.g. RadioGroup::column, RadioGroup::row, and RadioGroup::for_axis) What do you think about that?

@twitchyliquid64
Copy link
Contributor Author

Sure thing! I'll get on it this weekend :)

@twitchyliquid64 twitchyliquid64 changed the title Implement RadioGroup::new_with_axis to allow horizontal radios Implement RadioGroup::{row,column,for_axis} to allow layout customization Mar 27, 2022
@twitchyliquid64
Copy link
Contributor Author

Done! please take a look :)

@jneem
Copy link
Collaborator

jneem commented Mar 27, 2022

Looks great, thanks! We'll definitely want a changelog entry too (which I can do today if you don't have time).

(Note that the links in the changelog aren't automatic; they're all defined at the bottom)

@twitchyliquid64
Copy link
Contributor Author

Done, thanks!

Copy link
Collaborator

@jneem jneem left a comment

Choose a reason for hiding this comment

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

Thanks for your patience! I have to wait for CI to finish, and then I'll merge.

@jneem jneem merged commit aa4b366 into linebender:master Mar 28, 2022
xarvic pushed a commit to xarvic/druid that referenced this pull request Jul 29, 2022
xarvic pushed a commit to xarvic/druid that referenced this pull request Jul 29, 2022
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.

2 participants