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

shell: Respect user's toolbar-style setting #207

Merged
merged 2 commits into from
Mar 12, 2017

Conversation

zeldin
Copy link
Contributor

@zeldin zeldin commented Jun 13, 2016

Atril should respect if the user wants another toolbar-style than "both-horiz" by checking the "toolbar-style" key of the "org.mate.interface" schema in GSettings.

@monsta
Copy link
Contributor

monsta commented Jun 20, 2016

Nice - but would be better to check the existence of the schema before using it. There are people who use Atril in other desktop environments.

Also this setting is completely ignored in GTK+3 build - they made it no-op at some point... IIRC it was in GTK+ 3.10.

@zeldin
Copy link
Contributor Author

zeldin commented Jun 21, 2016

Right you are. I run it in XFCE myself, but I still have all the MATE schemas installed. 😄
I pushed a fix to this PR.

Also this setting is completely ignored in GTK+3 build

Hm? The patch works fine for me (as in, all the styles do what they are supposed to when I set the setting) with GTK+3.20.6 on Debian stretch...

@monsta
Copy link
Contributor

monsta commented Jun 22, 2016

I recall eom ignores this setting in GTK+3 build, so I thought Atril does the same... I'll check it once more.

@monsta
Copy link
Contributor

monsta commented Jun 22, 2016

Hmm, interesting... GTK+2 build of Atril reacts to this setting even without this PR. The only issue is that "icon" value isn't processed properly - Atril uses "both" value instead. GTK+2 build of eom has the same behavior. I'll check GTK+3 build later.

@raveit65
Copy link
Member

raveit65 commented Mar 3, 2017

@monsta
What is the state of merging this PR?
Or do we need this with gtk3 build any more?

@monsta
Copy link
Contributor

monsta commented Mar 10, 2017

Ok, it works with current git master. I guess it's because we have a custom "libegg" toolbar, not some standard GTK+ one. I remember the toolbars settings were ignored in the standard one, therefore we removed them from the last tab of mate-appearance-properties...

@monsta
Copy link
Contributor

monsta commented Mar 10, 2017

GTK+2 build of Atril reacts to this setting even without this PR. The only issue is that "icon" value isn't processed properly - Atril uses "both" value instead.

Here in GTK+3 code we get the same behavior with this PR. In other words, it seems ok since it restores the old behavior. As for some values not working, I guess it's some separate issue in the toolbars.

@monsta monsta requested review from raveit65 and lukefromdc March 10, 2017 11:06
@raveit65
Copy link
Member

hmm, it seems to be a bit inconsistent.

  • both --> text is under icons on most buttons
  • icon --> i see only icons, but for forward/previous buttons i see icons and text.
  • both-horz --> same as with icons

Copy link
Member

@lukefromdc lukefromdc left a comment

Choose a reason for hiding this comment

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

This works as advertised with GTK 3.22.9

@monsta
Copy link
Contributor

monsta commented Mar 11, 2017

Yes, this inconsistency is what I was talking about. You can check GTK+2 build (in 1.16) and see it there as well. That's why I think it's some separate issue in the toolbars.

@raveit65
Copy link
Member

As, this doesn't work for normal Gtk Toolbars i am in doubt that i will ever use that ;-)
I don't like to use all the old deprecated gtk+2 stuff for myself........
So feel free to merge it if you think it is OK.

@monsta monsta merged commit 5a15fc2 into mate-desktop:master Mar 12, 2017
@monsta
Copy link
Contributor

monsta commented Mar 12, 2017

Ok, rebased and merged.

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.

4 participants