gui: add Bootswatch themes#2892
gui: add Bootswatch themes#2892norgeous wants to merge 3 commits intosyncthing:masterfrom norgeous:bootswatch
Conversation
|
also, there might be an issue with these css files including online fonts from google... |
|
We should probably prefix them with "Bootswatch foobar" or something, because other themes could be more prominent and actually change how the UI is laid out rather than just colours. |
|
good idea |
|
lemme try this out |
|
@st-review merge this please gui: add Bootswatch themes |
|
Thanks @AudriusButkevicius |
|
original inspiration @marclaporte #1925 |
|
I don't think this is the right way to include these themes. As they replace the bootstrap.min.css file they are loaded "underneath" both overrides.css and theme.css from the default theme. For example, we override the header font no matter what the theme says. As I understand our theme mechanism, these should be theme.css files? |
|
Yes, potentially. |
|
I personally don't, although it's not particularly clean, really. At the very least the themes will look different with and without internet access, and it's potentially a tracking vulnerability (the people hosting the fonts get to see from the referrer where you're running Syncthing)... So yeah, probably not a super idea. |
|
Ok. I'll try to fix this before we release. |
|
Actually, when I'm now shifting the brain into paranoid mode, I think we should replace our minified bootstrap and angular dependencies with non-minified versions, and likewise not allow minified themes in either. I want to be able to audit these things, ideally automatically to look for imports/references to external things but at the very least manually. I doubt the performance difference in using non-minified versions matter for us. If it does, we could look into adding a minification step to the build. But it would be a disaster if we let something in that adds a javascript to compromise the user's browser so that it sends credentials to some third party, or even just lets someone track our users as noted above. We'll have to accept "on faith" that Bootstrap and Angular themselves aren't compromised when downloaded from a trusted source, as we'll probably not vet it all ourselves, but still. |
|
So the un-minified themes are 6000 lines each, I am not in a position to review them, hence I guess we are reverting? |
|
For CSS we'd only need to review them for external resources, i.e. anything like |
|
Well if we are not bundling the fonts and we are not happy hitting Google for them, then the only option is to revert. |
|
Do all of them load external fonts? Otherwise just delete the ones that do for now? |
|
Only 2 out of 16 don't. |
|
FWIW, if we do https://forum.syncthing.net/t/rework-build-process-to-avoid-repo-bloat/7054, I think it may be more realistic to include the fonts. Still requires rewriting those themes though |
st-review
added
the
frozen-due-to-age
4 participants
Attempt to integrate with bootswatch.com adding 16 themes to the project.
I don't know of a way to test (or properly integrate) additional themes (ie: have them show up in Syncthing > Actions > GUI Theme) as I have no GO development skills.
I tested a few of the themes manually (placing them as C:\Users\User\AppData\Local\Syncthing\gui\vendor\bootstrap\css\bootstrap.min.css), and they worked quite nicely. I believe the files should be placed in the way that I have done in the PR, but I don't know any way of completing the integration. Any takers?
All css files included in this PR are copied from https://bootswatch.com.