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

No lightweight way to adjust ui style #17

Closed
vkkoskie opened this issue Aug 16, 2020 · 2 comments · Fixed by #18
Closed

No lightweight way to adjust ui style #17

vkkoskie opened this issue Aug 16, 2020 · 2 comments · Fixed by #18
Labels
good first issue Good for newcomers

Comments

@vkkoskie
Copy link
Contributor

vkkoskie commented Aug 16, 2020

I could be wrong about the following. I'm just starting to learn Rust, so I could have misunderstood something. I believe this is the only way to change style elements given the current API:

let mut style = ui.style().clone();
style.item_spacing = egui::vec2(x, y);
ui.set_style(style);

The ui's style member is private and the ui.style() method returns an immutable reference. So the only way to change a single style element is to clone the whole struct out, make the change, and pass ownership back into the ui. This is awkward to implement, and forces needless copies. It's also redundant with the clone that happens when a child ui is created. Touching a given ui's style won't affect global style settings.

The patterns in the Style::ui widget hints that some kind of public access is safe and may have been intended, but I can't find a pathway for it.

@emilk
Copy link
Owner

emilk commented Aug 17, 2020

Good point! I think allowing ui.style_mut().item_spacing = vec2(x, y); would make sense.

@emilk emilk added the good first issue Good for newcomers label Aug 17, 2020
@vkkoskie
Copy link
Contributor Author

vkkoskie commented Aug 17, 2020

Great! PR submitted. After submitting the initial comment, I noticed the same thing about ui.layout. The PR includes both changes. Sorry if that's bad form.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants