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

Themes: add support for user styles. #2499

Merged
merged 1 commit into from Nov 17, 2016

Conversation

@mkrautz
Copy link
Member

commented Aug 7, 2016

Mumble will now read and use a user.qss
file from the base path.

The content of the user.qss will
be added after the actual theme's stylesheet.

This allows users to make small tweaks to
the themes we ship.

It also allows developers to test small tweaks
without rebuilding Mumble.

Locations where Mumble will look for user.qss:
Windows: %APPDATA%\Mumble\user.qss
macOS: $HOME/Library/Application Support/Mumble/Mumble/user.qss
Unix-like: $HOME/.local/share/Mumble/Mumble/user.qss

@davidebeatrici davidebeatrici added the theme label Aug 7, 2016

@davidebeatrici

This comment has been minimized.

Copy link
Member

commented Aug 7, 2016

Nice feature!
I think that it would be better to use /Mumble/Themes/user.qss instead of /Mumble/user.qss 😉

@hacst

This comment has been minimized.

Copy link
Member

commented Aug 7, 2016

You never have to actually rebuild mumble to test changes as user provided themes take precedence over our built-in ones if they have they have the same name. I still appreciate how overriding single values regardless of theme in a central file might be useful in some cases though. I guess you felt the need to have this in your own theme experiments? Any other specific use-cases you had in mind? Accessibility?

LGTM though I think we should add some logging to hint at a user.qss being used. It's quite possible this will get (ab?^^)used for customization by clans or similar and we don't want to be wondering about bugreports on built-in themes themes that are actually caused by some user.qss .

@Kissaki

This comment has been minimized.

Copy link
Member

commented Oct 20, 2016

I agree with hacst. While this may be useful for development, it is prone to be misused to distribute/set skins - with more complicated installation than necessary as well.

@Kissaki

This comment has been minimized.

Copy link
Member

commented Oct 20, 2016

Not sure about the path. It’s not a theme. But it is overriding themes/working like a theme.
Putting it inside has the advantage of not cluttering the main folder with (more and more) stuff.

@mkrautz

This comment has been minimized.

Copy link
Member Author

commented Oct 20, 2016

I'll add a log entry when a user.qss is active.

I have no strong preference for the path of the file, but I'm leaning towards having it where the PR puts it now (not in Themes).

Themes: add support for user styles.
Mumble will now read and use a user.qss
file from the base path.

The content of the user.qss will
be added after the actual theme's stylesheet.

This allows users to make small tweaks to
the themes we ship.

It also allows developers to test small tweaks
without rebuilding Mumble.

Locations where Mumble will look for user.qss:
Windows:    %APPDATA%\Mumble\user.qss
macOS:      $HOME/Library/Application Support/Mumble/Mumble/user.qss
Unix-like:  $HOME/.local/share/Mumble/Mumble/user.qss

@mkrautz mkrautz force-pushed the mkrautz:user-qss branch from ada726d to c21518f Nov 13, 2016

@mkrautz

This comment has been minimized.

Copy link
Member Author

commented Nov 13, 2016

PTAL.

We now log when a user.qss is active.

QLatin1String("\n") +
userStylesheetContent
);

This comment has been minimized.

Copy link
@EmperorArthur

EmperorArthur Nov 14, 2016

Contributor

This code is almost identical to what's above.

Have you considered moving both of them into a single function? It might help maintainability and readability.

This comment has been minimized.

Copy link
@mkrautz

mkrautz Nov 17, 2016

Author Member

Yes, but I think I'll log this as an issue, to keep this PR focused on adding user style support, not refactoring.

@mkrautz mkrautz merged commit 09a0f84 into mumble-voip:master Nov 17, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.