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

Use the system theme on Linux #10

Merged
merged 4 commits into from
Nov 13, 2017
Merged

Conversation

flying-sheep
Copy link
Collaborator

@flying-sheep flying-sheep commented Jan 24, 2017

This PR avoids the pitfalls of #5.

The default behavior is that the icons are taken from the theme if a theme is set (i.e. on Linux), and to load and use a builtin theme from the qrc file if not.

With this PR, the application icon will always be loaded in the “hicolor” fallback theme, so it’s used on Linux as well.

The default behavior can be overridden by setting the NBMANAGER_IGNORE_THEME environment variable to something. Then the builtin theme will even be used on Linux. (good for testing it or so)

uic2.py

There was a bit of feature creep since I had to adapt this script to do more (generating the changed paths and so on).

But it’s worth it! Now the only source of truth is the file hierarchy in the icons/ directory and the .ui file. They’re used to recreate the .qrc file, its compiled version, the index.theme files, and the OSX icon directory (which is no longer checked in)

Alternatives

  • It would also be possible to check if all icons are available in the system theme and using the builtin one if not. That’s uncommon behavior though.
  • It could be a build-time switch to include the builtin theme at all. Many distributions probably would like to disable that when packaging this application (and instead just put the application icon into /usr/share/icons/hicolor)
  • Start using uic2.py in setup.py to avoid checking in volatile files

@flying-sheep flying-sheep reopened this Jan 30, 2017
@flying-sheep
Copy link
Collaborator Author

huh, why did i close this?

@flying-sheep
Copy link
Collaborator Author

hi @takluyver what do you think?

@takluyver
Copy link
Member

I think your persistence will inevitably outweigh my will to argue. :-P

I'm concerned that uic2.py has got several times longer. It was only meant to be a simple script to convert a couple of files, I'm worried that we're now reimplementing a build system badly. Is there a better way to do what you're trying to?

Start using uic2.py in setup.py to avoid checking in volatile files

Yes please, especially as this PR currently adds ~11k lines in the compiled resources file.

@flying-sheep
Copy link
Collaborator Author

flying-sheep commented Mar 2, 2017

I think your persistence will inevitably outweigh my will to argue. :-P

haha i was actually unaware that you wanted to argue. i thought you were just dissatisfied with my previous design (#5) that didn’t incorporate a proper fallback icon theme.

I'm concerned that uic2.py has got several times longer. It was only meant to be a simple script to convert a couple of files, I'm worried that we're now reimplementing a build system badly. Is there a better way to do what you're trying to?

hmm, i see what you mean. it went from a relatively generic compiler to a pretty involved thing.

however, it’s not too project-specific. i think just spend quite some lines for creating an icon theme. as you’re the maintainer of pyxdg: do you think using it to create the theme .ini file would make our life easier here?

@takluyver
Copy link
Member

haha i was actually unaware that you wanted to argue. i thought you were just dissatisfied with my previous design (#5) that didn’t incorporate a proper fallback icon theme.

It's just a different approach to visual design - I approached it in more the web style, where the developer decides what the user will see; you're looking at it from the Linux desktop point of view, where the developer specifies a semantic category and the system finds an icon for that in the user's theme. I don't feel strongly about this, though.

however, it’s not too project-specific. i think just spend quite some lines for creating an icon theme. as you’re the maintainer of pyxdg: do you think using it to create the theme .ini file would make our life easier here?

I don't know if it has much to support that. I don't do much with pyxdg; I adopted it mostly to move Python 3 support forwards. Have a look if it can simplify what we're doing, but if not, it's not all that long.

@flying-sheep
Copy link
Collaborator Author

flying-sheep commented Oct 20, 2017

I approached it in more the web style, where the developer decides what the user will see; you're looking at it from the Linux desktop point of view, where the developer specifies a semantic category and the system finds an icon for that in the user's theme.

my feelings about this: there’s a few applications that i allow to do their own things, namely ones that are the subject of all of your focus for long times:

  • IDEs & extensible text editors
  • image/video viewers/editors
  • browsers

everything smaller, everything you interact with only for seconds at a time, should be unobtrusive, blend in, and try to adhere to your system’s conventions as closely as possible.

@takluyver takluyver merged commit 7336a6b into jupyter:master Nov 13, 2017
@flying-sheep flying-sheep deleted the iconset branch November 13, 2017 16:15
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.

None yet

2 participants