Skip to content
This repository was archived by the owner on Dec 16, 2022. It is now read-only.

Add theme selector to settings #454

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

floogulinc
Copy link
Collaborator

Adds a theme selector to the settings that allows users to choose a different theme. Themes are managed in config.js and consist of only a CSS file. The default config.js now comes with the default theme and the other 3 that are come in the themes folder preconfigured.

After updating, admins should either add the new theme stuff and remove the old variable, or regenerate a new config.js.

Closes #452 and #90.

screen shot 2015-08-05 at 04 25 08

fernviridian pushed a commit to fernviridian/shout that referenced this pull request Aug 21, 2015
@richrd
Copy link

richrd commented Sep 24, 2015

👍 I would love this! Although I feel that the theme list should be automatically detected from the theme folder. I like to create my own themes or modify existing ones after copying them to a different file. I understand this requires extra hassle on the server side, but I think detection would be optimal solution and offers the best user experience.

That said, I still feel this could be merged any time :)

@floogulinc
Copy link
Collaborator Author

Well, adding themes with this setup only requires adding a few lines to the config file for your instance, It also allows for the theme file to be at any path, or even a URL to one hosted elsewhere.

It looks like this:

{
    "name": "Crypto", //Theme name displayed to user
    "path": "themes/crypto.css", //Theme path/URL
    "default": false //Should the theme be the default for new users
}

This also would allow for a feature in the future to allow users to install their own themes on the frontend just from a URL.

@richrd
Copy link

richrd commented Sep 24, 2015

Ok, looks good. For my requirements I just need config reloading after this is implemented 👍

@heubergen
Copy link

FYI: Works for me, I've updated my files manually.

@astorije astorije self-assigned this Dec 20, 2015
@@ -214,12 +214,26 @@ <h1 class="title">Settings</h1>
<div class="col-sm-12">
<h2>Visual Aids</h2>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would prefer a new "Themes" category rather than adding this to "Visual Aids".

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@astorije Note that in Shuo we renamed this area "Visuals" in a different PR.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for the precision! I would use the word "Theme" though, or "Interface" maybe, but I prefer "Theme". And we'll include there everything related to design/look options.
What do you think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@astorije I honestly like calling it "Visuals" because it is specifically things relating to the visual appearance.

@astorije
Copy link
Collaborator

astorije commented Jan 6, 2016

Hey @floogulinc, thanks! I left a few comments inline, but I think we are very close to a shippable solution overall, great! :-)

@floogulinc
Copy link
Collaborator Author

@astorije with all the whitespace stuff, I was just following what was already used in the rest of the file.

@astorije
Copy link
Collaborator

astorije commented Jan 8, 2016

Hey @floogulinc, actually, most of the code is func(arg1, arg2) and not func(arg1,arg2) so let's keep that version. :-)
Also, I might have been unclear in my last comment, but the problem is some lines of defaults/config.js are using spaces for indentation where they should be using tabs as the rest of the project.

I can't wait to see this released!! :-)

@floogulinc
Copy link
Collaborator Author

@astorije OH, thats what you meant, I thought you were talking about the indenting. I'll make some changes when I have the time. I'm going to be quite busy for the next 6 weeks or so.

@astorije
Copy link
Collaborator

Argh, it's a shame, this is so close to a shippable solution :-(

@astorije
Copy link
Collaborator

Noticed a potential duplicate: #386
I'll review and check if they are indeed duplicates or not.

@AlMcKinlay AlMcKinlay mentioned this pull request Jan 24, 2016
@astorije astorije removed their assignment Mar 11, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants