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

Handle IsSameLine in Separator() #7360

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ostef
Copy link

@ostef ostef commented Mar 2, 2024

This is a simple one, but I can't find any reason IsSameLine shouldn't be handled in Separator.

One thing I am not sure about is the SameLine() call at the end of Separator if we used SameLine() before. Although I don't see a practical reason to not restore IsSameLine state, it feels weird that specifically Separator wants to keep things on the same line even though all the other widgets do not (that I am aware of).

I haven't tested this in weird cases, are there edge cases maybe that I should be aware of?

@ocornut ocornut added the layout label Mar 5, 2024
@ocornut
Copy link
Owner

ocornut commented Mar 5, 2024

This is a simple one, but I can't find any reason IsSameLine shouldn't be handled in Separator.

I don't think it's a simple one, as "how to handle it" is subject to design and intuitively I didn't expect you to use an vertical separator for this case, but merely to adjust the start X1 position of the horizontal line. So I was surprised by the content of the PR, but it means there's at least two different ways to do things, if not more.

It would be good to have more context about your use case, but perhaps the better answer for it is to expose a VSeparator() function or the separator flags.

Although I don't see a practical reason to not restore IsSameLine state, it feels weird that specifically Separator wants to keep things on the same line even though all the other widgets do not (that I am aware of).

Yes that part would be inconsistent. However tedious it is to call SameLine() an extra time, consistency is better way.

I would argue we should further push to make horizontal layout mode a public API (along with other planned layout mode but the others will requires bigger code change).

@ostef
Copy link
Author

ostef commented Mar 5, 2024

Well the reason I didn't make a separate function or expose the flags was that I was assuming a vertical separator was the intended behavior in this case, and it not being handled was an oversight.

I didn't expect you to use an vertical separator for this case, but merely to adjust the start X1 position of the horizontal line.

I don't really understand what you mean by that. Do you mean having a small horizontal separator like so A B C - D instead of A B C | D ?

So it's clearer here's what I wanted to achieve:
sameline_separator

Using ImGui::SameLine (); ImGui::Separator (); ImGui::SameLine () as it is now, I was surprised by the result:
weird_separator

I would argue we should further push to make horizontal layout mode a public API (along with other planned layout mode but the others will requires bigger code change).

I think so too, it's tedious to put SameLine calls all over the place when trying to layout things horizontally (although maybe I'm just not using the correct API for this).

@ocornut
Copy link
Owner

ocornut commented Mar 5, 2024

Do you mean having a small horizontal separator like so A B C - D instead of A B C | D ?

I meant A B C ----------- with the line starting from edge of last item when using SameLine().

So it's clearer here's what I wanted to achieve:

Right. This is currently what SeparatorEx(ImGuiSeparatorFlags_Vertical) would do but it's not exposed in public API as of yet.

If the behavior above ("A B C ----") doesn't work or is hard to make use it, that gives us some flexibility to decide that a default could be dynamically switching depending on context. The work here is mostly to consider many use cases and design an API as future-proof as possible.

think so too, it's tedious to put SameLine calls all over the place when trying to layout things horizontally (although maybe I'm just not using the correct API for this).

I don't disagree, but same problem: like with many unfinished features the difficult is designing the future proof API.
I usually need to design a proof of concept for more sub-features before I can expose a subset of them as public. Layout wise we'd want auto-wrapping, centering on the other axis, hook for custom layout systems etc. so whatever entry point need to be considerate of that. But in the meanwhile you can manually use window->DC.LayoutType = ImGuiLayoutType_Horizontal to get there.

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

Successfully merging this pull request may close these issues.

2 participants