RhetTbull / guitk

Python GUI Toolkit for Tk (guitk): simplify the layout and construction of tkinter graphical user interfaces in python using a declarative syntax.
MIT License
17 stars 1 forks source link

Allow stacks to expand independently by axis #50

Closed kevinrpb closed 2 months ago

kevinrpb commented 2 months ago

Hi there!

I was checking out this library and it's awesome! Thank you so much for making it 😄

There are a couple things that I wished I could do but couldn't. This one in particular seemed straightforward enough to implement, so here's a PR!


This adds support for passing vexpand/hexpand to *Stacks and *Grids to control the 'expansion' on each axis individually. The main use case for me was having HStacks in a VLayout that expand horizontally but not vertically so their height is always the same.

The existing behavior with expand/height/width remains the same, so this change is backwards compatible. But if expand=True, then the new parameters can be used.

Let me know if there's anything I missed 😊

RhetTbull commented 2 months ago

Thanks! Glad you found this useful. I didn't know anyone else was actually using this code so I'm glad it was useful! This is a great idea. I wonder if it would make sense to just replace expand with vexpand and hexpand (with both True by default)? This might make the case of someone accidentally using expand=True, vexpand=False, hexpand=False less confusing. On the other hand, it would make the default case for most containers except grid different as you'd had to remember to use vexpand or hexpand as appropriate. I think your implementation is probably good, just thinking out loud.

RhetTbull commented 2 months ago

@all-contributors please add @kevinrpb for code

allcontributors[bot] commented 2 months ago

@RhetTbull

I've put up a pull request to add @kevinrpb! :tada:

kevinrpb commented 2 months ago

I didn't know anyone else was actually using this code so I'm glad it was useful!

@RhetTbull Yeah! Honestly I just found out the package a couple days ago, but it is super useful so far for some small utilities I'm working on!

I wonder if it would make sense to just replace expand with vexpand and hexpand.

Ah, that's a good thought. I'm honestly not sure. Since you are way more familiar with the package I think it makes sense for you to take that decision, both alternatives seem sensible to me 🙂

Just let me know and I'll update the PR as needed 👌🏽

RhetTbull commented 2 months ago

@kevinrpb after thinking about this some more I think it makes most sense to just remove the expand argument. With both vexpand and hexpand as default True, the effect is the same in the default case and your addition allows the expansion to be controlled independently in each direction. Would you update the PR to remove the expand argument?

kevinrpb commented 2 months ago

Yeah, I can update the PR. Will do that tomorrow 👍🏽

kevinrpb commented 2 months ago

@RhetTbull PR should be good now.

With the change I think the logic in _Stack:_create_widget could be simplified. Please, take a look to verify it if you have the change 🙂.

(PS: Also, feel free to squash the commits when merging. I pushed it into two different ones so I could install the package locally and test the last change.)

kevinrpb commented 2 months ago

Actually, I just realized I need to update some of the example/test files to remove uses of expand. Will do that and get back to you.

kevinrpb commented 2 months ago

@RhetTbull updated it 👍🏽

RhetTbull commented 2 months ago

Thanks! I've published the new release.