sugarlabs / sugar-artwork

Sugar icons and themes
Apache License 2.0
11 stars 33 forks source link

Create seperate css for gtk+ 3.20 (and therefore 3.19 as well) #86

Closed samdroid-apps closed 8 years ago

samdroid-apps commented 8 years ago

TL;DR Gtk3.20 breaks everything, changes selectors. This patch fixes some things, breaks others. Importantly, it adds a seperate css file for gtk3.20 and gtk-older-than-3.20 clients so that things can be specifically fixed.

Requires https://github.com/sugarlabs/sugar-toolkit-gtk3/pull/293 and https://github.com/sugarlabs/sugar/pull/645

Gtk 3.20 included a huge change, the movement to css nodes [1].

Css nodes change the selectors of the css in an annoying way that makes it very different from previous releases. Therefore, I have created a seperate css file so that sugar can remain themeing for older gtk3 versions (eg. 3.8 on OLPC OS 13).

[1] https://blogs.gnome.org/mclasen/2015/11/20/a-gtk-update/

See the screen shots from gtk3-3.19.8-2.fc24.x86_64

Before After
https://gyazo.com/5e07815ddc74f729c50a6e1bb6d34fd8 https://gyazo.com/d5f1a4d52ddcc60b985699275002454d
https://gyazo.com/5a035a94f422dbdcb557768473c690f6 https://gyazo.com/f6a521a00a433c5a29488383780f27f1
https://gyazo.com/feb6034846e5f140396ed164aaeda6c9 https://gyazo.com/449ccd16e4081eb8151a870411c25914
https://gyazo.com/0eac1f2f88b3cb08d72c566f7bbe4576 https://gyazo.com/ebfc77a780aea0bf861ba0017d183f99
samdroid-apps commented 8 years ago

Ah, I had a chat with Company on the gnome-design irc. Company answered some of the questions that (I) had related to this patch:

samdroid-apps commented 8 years ago

I have updated this patch to fix more of the regressions. I have also updated it to use the set_css_name function and now depend on sugar and toolkit changes (listed in the description).

I have not updated the screenshots. However, it seems to fix the 3 test cases satellit mentioned in his email. Note, it does not fix the issue with the separator styling, see https://bugzilla.gnome.org/show_bug.cgi?id=760018 for why

If anybody could test this on older gtk versions, that would be appreciated as this should not cause any regressions.

Happy new year by the way!

samdroid-apps commented 8 years ago

Another upstream ticket https://bugzilla.gnome.org/show_bug.cgi?id=760040

samdroid-apps commented 8 years ago

Chatting with @quozl on irc and he rightly so told me that this is more of a feature than a bug fix.

@tchx84 is this an acceptable time in the release for this change?

quozl commented 8 years ago

Typo in patches;

Not necessary to say the change in Gtk+ is huge or annoying, as these are value judgements peripheral to the commit.

The patch cascades a change to sugar-artwork.spec to add new files:

#gtk3.20
%{_datadir}/themes/sugar-100/gtk-3.20/gtk.css
%{_datadir}/themes/sugar-100/gtk-3.20/gtk-widgets.css
%{_datadir}/themes/sugar-100/gtk-3.20/assets/*
%{_datadir}/themes/sugar-72/gtk-3.20/gtk.css
%{_datadir}/themes/sugar-72/gtk-3.20/gtk-widgets.css
%{_datadir}/themes/sugar-72/gtk-3.20/assets/*

The patches fail on Fedora 18 during start of Sugar, and the desktop does not appear:

Traceback (most recent call last):
  File "/usr/lib/python2.7/runpy.py", line 162, in _run_module_as_main
    "__main__", fname, loader, pkg_name)
  File "/usr/lib/python2.7/runpy.py", line 72, in _run_code
    exec code in run_globals
  File "/usr/lib/python2.7/site-packages/jarabe/main.py", line 66, in <module>
    from jarabe.view import keyhandler
  File "/usr/lib/python2.7/site-packages/jarabe/view/keyhandler.py", line 33, in <module>
    from jarabe.journal import journalactivity
  File "/usr/lib/python2.7/site-packages/jarabe/journal/journalactivity.py", line 29, in <module>
    from sugar3.graphics.alert import ErrorAlert
  File "/usr/lib/python2.7/site-packages/sugar3/graphics/alert.py", line 55, in <module>
    from sugar3.graphics.icon import Icon
  File "/usr/lib/python2.7/site-packages/sugar3/graphics/icon.py", line 834, in <module>
    CanvasIcon.set_css_name('canvasicon')
AttributeError: type object 'CanvasIcon' has no attribute 'set_css_name'

Please code with the assumption that the class may not have an attribute set_css_name?

samdroid-apps commented 8 years ago

Thanks for testing.

I have forced pushed changed to only call to set_css_name if it is supported. Hopefully that fixes the issue.

quozl commented 8 years ago

After merge, build and install, seems to work on Fedora 18 without problems. Are there any specific tests to make?

samdroid-apps commented 8 years ago

Does the palette border look correct with tool bar button (probably incorrect name)? Eg. display device in frame?

quozl commented 8 years ago

Not quite sure what you mean. It seems fine. Here's what it looks like:

screenshot of _home_

samdroid-apps commented 8 years ago

Great!

samdroid-apps commented 8 years ago

Recently, gtk3-3.19.6-1.fc24.x86_64 made it's way onto my computer. This is a good release because it fixed 2 of the outstanding issues with this patch, namely that invisible separators were not invisible and that entries in toolbars had no padding!

samdroid-apps commented 8 years ago

Hello! I updated this patch for better testing on gtk3.19.8, and I have updated the screenshot above. Could we merge this going into the next cycle?

nullr0ute commented 8 years ago

sugar

I've applied this patch to sugar-artwork 0.108.0 in Fedora 24 with gtk3 3.19.9 but there's still some issues. See screenshot, in particular the top right. It would be good to get that fixed and then this landed for a 0.108.1 release.

samdroid-apps commented 8 years ago

Hum, that's how it is meant to look before the patch :)

Did you apply the sugar and toolkit patches as well (link in initial comment)?

Did the Gtk 3.20 theme version get installed (there should be a gtk3-20 or simmilar directory next to the normal gtk3 directory)? The makefile should do that.

tchx84 commented 8 years ago

@nullr0ute please try this again with the complementary patches as @samdroid-apps suggests, if it works for you we could definitely make the 0.108.1 happen.

nullr0ute commented 8 years ago

I just used the artwork patches I don't have time to pull patches from places. I NEED this stuff in supported releases, I really don't have the time to be randomly pulling patches from everywhere to make it work

samdroid-apps commented 8 years ago

@nullr0ute this patch needs to be tested before it is merged into sugar. I prefer to distribute my patches via git rather than release tarballs sorry. It's just for testing - I'm sure you are familiar with the process.

Feel free to test it if you want though. Your input would be very much appreciated.