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

ImSequencer: Don't expand to fill all space in the window #195

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

Conversation

idbrii
Copy link
Contributor

@idbrii idbrii commented Aug 7, 2021

Instead of expanding to fill available space, calculate the space we need and use that much.

If you have code roughly like:

    Sequencer(&mySequence, &currentFrame, &expanded, &selectedEntry, &firstFrame);
    float color[4];
    ImGui::ColorEdit4("Color", color);

you get blank space below imsequencer and can't see the widgets below it:
image

With this change, you can see the widgets below it.

image

However, the downside is that the thumb can scroll off the bottom of the window:

image

@idbrii idbrii changed the title Don't expand to fill all space in the window ImSequencer: Don't expand to fill all space in the window Aug 7, 2021
@CedricGuillemet
Copy link
Owner

Not sure about this one. You can set Imsequencer height from the available content height (minus your other controls).

@idbrii
Copy link
Contributor Author

idbrii commented Aug 9, 2021

Is there a case where expand to fill is preferable?

You can set Imsequencer height from the available content height (minus your other controls).

Do you mean wrap Sequencer in Begin/EndChild? Doesn't that mean I need to determine the height of my widgets below ImSequencer so it can expand appropriately? Awkward when using a bunch of CollapsingHeader:

static bool was_expanded = false;
ImVec2 sequencer_size = ImGui::GetContentRegionAvail();
sequencer_size.y -= 20;
if (was_expanded) {
    sequencer_size.y -= 25 * 2;
}

ImGui::BeginChild("Sequencer", sequencer_size);
{
    Sequencer(&mySequence, &currentFrame, &expanded, &selectedEntry, &firstFrame, ImSequencer::SEQUENCER_EDIT_ALL);
}
ImGui::EndChild();

was_expanded = ImGui::CollapsingHeader("Colors");
if (was_expanded) {
    float color[4];
    ImGui::ColorEdit4("Color", color);
    ImGui::ColorEdit4("Another Color", color);
}

And if I have lots of controls underneath, then imgui adds scrolling and ImSequencer doesn't clip properly while scrolling (probably fixable in master by just taking 6f60505):

image

With this PR, you don't get that clip badness. You will get double scrollbars, but I'm not sure why.

image


Regardless, assuming the normal use of ImSequencer is selecting an item and then using additional ImGui widgets below the sequencer to display data for that item, I'd think you want that flow it be easiest -- automatically figure out our height so widgets below will display.

Fix sequencer expands to fill the space its in and pushing other
elements off below it.

The sequencer doesn't need to fill the space and it's likely that you
want to draw additional information about the selection below it.
Instead of filling available space, fill horizontal space and calculate
how much space we need.
@idbrii
Copy link
Contributor Author

idbrii commented Oct 26, 2022

Rebased on master now that some of the old commits in here are merged and the issues from my previous comment are fixed.

"the downside is that the thumb can scroll off the bottom of the window" is still an issue and that's probably the main blocker to getting this merged. Doubtful I'll have time to address that soon.

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