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

Allow resizing of panes in PanedBrowser #2301

Merged
merged 5 commits into from Mar 14, 2017
Merged

Allow resizing of panes in PanedBrowser #2301

merged 5 commits into from Mar 14, 2017

Conversation

@frestr
Copy link
Member

@frestr frestr commented Mar 6, 2017

Implements #1118

The way it is implemented is to use a nested paned structure instead of the existing vbox. For example, for three panes - p1, p2 and p3 - the structure will look like Paned(p1, Paned(p2, p3)). Gtk doesn't seem to have paneds with more than two panes, so I'm not sure if this can be solved in any other way.

The pane widths are made persistant by extension of the paneds being instances of ConfigRHPaned. ConfigRHPaned only supports writing its width to a single option variable in the config though, so to enable an arbitrary number of paned widths to be saved, each width is saved in the config under the "browser_paned_widths" section, under options "0", "1" etc.

I'm not sure if this is the preferred way to do it, as I couldn't find anything similar already in the code. But as I see it, the alternative will be to either have an arbitrary number of widths saved under the "browsers" section (not good imo), or to save them as a single list option. The disadvantage with the latter will be that ConfigRHPaned needs to be changed, and I also don't think the paneds can constantly write their widths like they do now if they have to all write to the same option (so it may need to be changed to only write widths on shutdown / browser change). Any input on this?

There's also a checkbox in the paned browser preferences - "Equal pane width" - which is checked by default. When checked, all panes will be set to have equal width when the apply button is clicked, so basically like it has been up until now. The reason for having that checkbox is in case people don't want their pane widths to be changed after adding a new pane.

frestr added 2 commits Mar 6, 2017
Fixes quodlibet#1118

* Implemented by having a nested paned structure instead of a static
  vbox
* Includes a checkbox in the preferences to make the panes equal in
  width
@lazka
Copy link
Member

@lazka lazka commented Mar 7, 2017

The way it is implemented is to use a nested paned structure instead of the existing vbox. For example, for three panes - p1, p2 and p3 - the structure will look like Paned(p1, Paned(p2, p3)). Gtk doesn't seem to have paneds with more than two panes, so I'm not sure if this can be solved in any other way.

There is a multi paned in C: https://github.com/chergert/panel-gtk/blob/master/src/pnl-multi-paned.c
or one could wrap your strategy in a new container widget, hiding all the ugly details. But all in all it looks good. If we need it in more places we definitely have to look into cleaning it up.

The pane widths are made persistant by extension of the paneds being instances of ConfigRHPaned. ConfigRHPaned only supports writing its width to a single option variable in the config though, so to enable an arbitrary number of paned widths to be saved, each width is saved in the config under the "browser_paned_widths" section, under options "0", "1" etc.

ok. hacky, but works :)

I'm not sure if this is the preferred way to do it, as I couldn't find anything similar already in the code. But as I see it, the alternative will be to either have an arbitrary number of widths saved under the "browsers" section (not good imo), or to save them as a single list option. The disadvantage with the latter will be that ConfigRHPaned needs to be changed, and I also don't think the paneds can constantly write their widths like they do now if they have to all write to the same option (so it may need to be changed to only write widths on shutdown / browser change). Any input on this?

Ideally we'd have a new widget (not ConfigRHPaned) which would give you a list of widths which can be saved if any change. Re save on shutdown: we don't have any event to save atm (destroy is too late) so we always save everything on change.

There's also a checkbox in the paned browser preferences - "Equal pane width" - which is checked by default. When checked, all panes will be set to have equal width when the apply button is clicked, so basically like it has been up until now. The reason for having that checkbox is in case people don't want their pane widths to be changed after adding a new pane.

What about a "Reset pane widths" button instead of the checkbox?

@frestr
Copy link
Member Author

@frestr frestr commented Mar 7, 2017

There is a multi paned in C: https://github.com/chergert/panel-gtk/blob/master/src/pnl-multi-paned.c
or one could wrap your strategy in a new container widget, hiding all the ugly details. But all in all it looks good. If we need it in more places we definitely have to look into cleaning it up.

Nice, I wasn't aware of that. It seems like it has not been integrated into Gtk yet though, but is marked as a prototype on the Gtk roadmap.

Ideally we'd have a new widget (not ConfigRHPaned) which would give you a list of widths which can be saved if any change. Re save on shutdown: we don't have any event to save atm (destroy is too late) so we always save everything on change.

Sounds like a good idea. I'll try to encapsulate the functionality in a new widget.

What about a "Reset pane widths" button instead of the checkbox?

I actually had that in the beginning, but found it rather tedious to always have to click both apply and that button when changing the panes if I wanted the panes to have the same width. A button may be a little more intuitive though.

@zsau
Copy link
Contributor

@zsau zsau commented Mar 7, 2017

I actually had that in the beginning, but found it rather tedious to always have to click both apply and that button when changing the panes if I wanted the panes to have the same width. A button may be a little more intuitive though.

FWIW, the way it works for columns in a TreeView is that any column the user resizes gets its width saved (becoming fixed-width), while other columns' widths remain unspecified/undefined. Then the "extra" view width (total width minus all fixed-width columns) is divided equally among the non-fixed-width columns.

@frestr
Copy link
Member Author

@frestr frestr commented Mar 12, 2017

I've added two classes, MultiRPaned and ConfigMultiRPaned (plus respective "orientation" versions), mostly mimicking the style of RPaned and ConfigRPaned. ConfigRPaned saves the widths in a string list, so now only a single option field is needed. Thoughts?

@urielz
Copy link
Contributor

@urielz urielz commented Mar 13, 2017

@frestr this is awesome! I really wanted this in QL. Thank you.

@lazka
Copy link
Member

@lazka lazka commented Mar 13, 2017

Looks good

@lazka
Copy link
Member

@lazka lazka commented Mar 14, 2017

Is there anything missing or should I merge?

@frestr
Copy link
Member Author

@frestr frestr commented Mar 14, 2017

Everything should be in place, so go ahead.

@lazka lazka merged commit a63d486 into quodlibet:master Mar 14, 2017
1 check passed
@lazka
Copy link
Member

@lazka lazka commented Mar 14, 2017

Thanks

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

Successfully merging this pull request may close these issues.

None yet

4 participants