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

CollapsingState Improvements #1569

Closed
wants to merge 3 commits into from
Closed

Conversation

4JX
Copy link
Contributor

@4JX 4JX commented May 4, 2022

As per #1566

Concerns are the FnOnce > Fn change, which shouldn't realistically impact any icon drawing functions and the ergonomics of a "no collapser" mode, since something like 4JX@f3234a1 would need to be done.

@4JX 4JX marked this pull request as ready for review May 4, 2022 17:34
@4JX
Copy link
Contributor Author

4JX commented May 4, 2022

Would like to also add the commit mentioned above to this PR as well if it isn't deemed that much of an issue

Comment on lines +202 to +203
ui.spacing_mut().item_spacing.x = 0.0; // the toggler button uses the full indent width
let collapser = self.show_default_button_indented(ui);
Copy link
Owner

Choose a reason for hiding this comment

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

what does it mean to match the indentation in this case?

@emilk
Copy link
Owner

emilk commented May 4, 2022

My intent for CollapsingState was to make it more of a building block, and not something you customize. I would rather see a solution that looks something like this (from a users perspective):

let mut collapsing_state = …;
let header_response = ui.horizontal(|ui| {
    ui.label("Header");
    collapsing_state.show_toggle_button(ui, my_icon_painter);
});
collapsing_state.show_body(&header_response.response, ui, |ui| {
    ui.label("Body");
});

This keeps CollapsingState simple, and makes for a much more powerful and flexible solution.

@4JX
Copy link
Contributor Author

4JX commented May 4, 2022

This keeps CollapsingState simple, and makes for a much more powerful and flexible solution.

Oh, I was unaware a state could be constructed with an arbitrary response/header, hence the approach of this PR. I'll close it and draft a new one with the approach you described, as it seems a lot more useful.

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