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
Make display of window buttons in top bar user configurable #70
base: master
Are you sure you want to change the base?
Conversation
showButtons(); | ||
} | ||
})); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As per your request, the wiring now happens in the buttons module.
If I understood you correctly, the options should also be loaded here. This implies that we have to perform cleanup as well. Generally not a problem, but as more modules use the options, they all have to perform cleanup if they listen to settings changes.
I would therefore suggest that at least the option loading should move to the Extension module as then cleanup has to performed only once (and you don't have to remember the constraint to cleanup options if you listen for changes).
For some reason i missed the notification that the PR was updated. So thanks @gsantner . i'll be reviewing this over the week end. |
Is there a chance to get this merged? 👍 |
Mmmmh so there was another very similar PR I got confused. @gsantner I want this actually. I'm just swamped. |
Would love to see this get merged |
This is very similar to #53 and same comment apply. This is a kitchen sink: add 3rd party code, add translation support, add a build system, add configuration screen, and add the feature for the buttons. The reason this is hanging for so long is that i only have limited time to review this, and this is made is such a way that I simply can't review it with limited time, because of the above. So I propose this is split is such a way: This is better done as separate PRs. I'll focus on 1/ so far. There are 4 options, but 2 of them apply on left side buttons. There is already some support for button on the left side, if gnome's configuration says so. Therefore, I don't think these options makes sense for right hand side buttons. |
Option to remove the buttons completely makes real sense: closing can be done with the drop down menu on the title, (un)maximizing by draging the window, and of course there are keyboard shorcuts for all that. So there is really no need for these buttons, is there ? |
But then, you don't need these buttons either on every window and can remove them from your gnome config. |
@deadalnix Well, I didn't think of that ! :-) But, then when not maximized, I have no close option (expect kb shortcuts), since it's added in the drop down menu in the application's title when maximized only. |
Sorry for the spam, but this might be useful to some. A fork including this PR is available on e.g.o: https://extensions.gnome.org/extension/1267/no-title-bar/ |
I had some problems with git and needed to reset master. It was easier then to create a new PR. Sorry for that.
Here is another one that provides the same, minus localization, but plus the extended set of button position options. Tested on 3.18.
Tab width has been set to '2'. I assume this is your preference?
fixes #35
fixes #46