Skip to content

Implement min size constraint for splitter #613

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 4 commits into from
Mar 10, 2020

Conversation

pyroxymat
Copy link
Contributor

@pyroxymat pyroxymat commented Mar 8, 2020

I am not sure about the API still for the split constraints. But this is a draft of how to implement them. You can check it out in the "split_demo" example.

Closes issue #540

Copy link
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

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

I agree this API is a bit tricky, and I'm honestly not sure if I can think of anything better.

The best idea I have is still the one I mentioned in the linked issue; just have a single min_size that is shared between both sides of the split. This might not be perfect, but I do think it probably solves 90% of cases?

@pyroxymat
Copy link
Contributor Author

pyroxymat commented Mar 8, 2020

Okay. I simplified the API additionally, I changed the splitter demo to only have a single splitter. Since it doesn't make a lot of sense to have a nested splitter in case the min sizes are really off (the min size for the right 2 is together the same as the min size for the left split).

I guess a good solution at some point might be to move to a splitter where you can have multiple splits? This is how QT does it as far as I know.

@pyroxymat pyroxymat marked this pull request as ready for review March 8, 2020 22:51
@Zarenor
Copy link
Collaborator

Zarenor commented Mar 9, 2020

I think we should index the panes left-to-right and top-to-bottom. So to min-size them independently, we have first_min(min_first?) and min_second. I could imagine max-sizes too, but I'm not sure how we handle the possibility of being asked to split a container too large for our maximum sizes if they're both/all set.

I had gone for 'simple, fast, and composable' with the single-splitter-only design. It handles, I think, the 80% case and has much simpler code than a multiway split. I also think filling out the constraints (mins and maxes) makes having to split several times more tolerable. I think being able to set the split point in units (pixels?) gets us the rest of the way to workable.

However, I wouldn't be opposed to creating a separate multisplitter, or altering this one with a vec of parameters, and so an arbitrary number of splits. Whichever we think is more appropriate. I think in that case agreeing to indexing in a direction would matter.

Copy link
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

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

A couple little notes, but looks good!

Copy link
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

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

Looks good, thanks. I have one doc fix I'll apply before merging, trust you don't mind.

@cmyr cmyr merged commit 4d3c240 into linebender:master Mar 10, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants