-
Notifications
You must be signed in to change notification settings - Fork 239
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
add nord #151
base: master
Are you sure you want to change the base?
add nord #151
Conversation
This update adds the modified theme from Weilbyte#143 as a branch to the main repository Co-authored-by: didli <[email protected]>
Hi @b-
Do you mean these icons ? |
It doesn't look like that on mine, no: I added you as a collaborator to my repository if you want to edit the branch that's attached to this PR directly ( |
By the way, I think it's pretty alright for legibility! Maybe not quite as much so as the stock or upstream PVEDiscordDark themes, but it's a great start! I think this variant is really awesome, but even more importantly I like the idea of lowering the barrier to making more custom themes — I figure if most custom themes are just recolors of this one (perhaps also swapping images) it shouldn't be too difficult to create some simple tooling for adding more theme variants. I'm also really not married to the idea of different themes living in different branches. It's probably ideal for development, but it would probably be easier for end users to customize things and create their own variants if it's all in one tree now that I think of it. Forgive me for being unclear — I'm not sure how to really explain this — but maybe we could replace Relatedly, I'm thinking it might make sense to modify the installer to work slightly differently with regards to offline installation. Presently, the installer checks for the presence of a directory $ git clone https://github.com/Weilbyte/PVEDiscordDark
$ cd PVEDiscordDark
$ sudo ./PVEDiscordDark.sh install We could also add a I should put the above into their own issues and PRs, and I probably will at some point, but I'm tired and feeling a little lazy right now so I think I'm going to stop typing and hit the "Comment" button instead. I want to be clear, though, that when I say "we," I mean "anyone interested in working on this" since it's an open source project on Github. I probably will do some of this when I feel up to it. |
First, thank you for your involvement ! These are some neat ideas that I would like to see. Maybe some will end up in the official proxmox dev git one day (here)
Since I don't have much knowledge about github (my own repo is something I did on a whim and forgot all about it until now), I can't clone your branch right now since I don´t know how to get allowed to do so. |
It doesn't really, because it looks to me like you edited the generated CSS and not the files which are used to generate the CSS? That makes it a lot harder to update, or to merge into the repo :( Let me see what I can do to make it easier to get a working dev environment for this. |
@didli Here, follow these steps, and then do not edit the css files directly. Instead, edit the sass files, and run the command in step 7 whenever you're done. https://github.com/b-/PVEThemes/tree/nord-develop Note: substitute |
Please make this theme compatible with PBS (Proxmox Backup Server)! |
Hi, please see #136 and specifcally the second-to-last comment. There is a fork of the Discord Dark theme for PBS by another user on github (https://github.com/Luckyvb/PBSDiscordDark). I don't think PBS is nearly as popular as Proxmox VE, so I don't expect it to be maintained as much as this one (and @Weilbyte has stated they aren't planning to support PBE themselves). You're more than welcome to fork that and update it and/or add the Nord colors, though! I'm sure a lot of people would appreciate it! |
Hi, thank you for the very in-depth PR. The general idea of custom themes initially was to have something like a GitHub wiki which lists forks which have been modified with different colors. However, when giving this more thought it is not as secure since providing the installer a custom REPO and TAG essentially means that the JavaScript file will also be installed, thus if the fork has replaced the harmless code with malicious code, it will be executed. While on this topic, I am not aware of why supplying the REPO/TAG did not work for you, could you please try instead using the SHA-1 hash for the tag/branch instead? As it stands the code is a bit of a mess, so when I find time I am going to first look if it can be restructured in a better way. If not, then a rewrite of the tooling (from shell script and python to single static executable) would take priority before themes. With this tool rewrite however, it would be feasible to have the theme compile locally, which should hopefully then allow more flexibility in custom themes (i.e., swap out vars file before compile, according to user selection). Let me know what you think of this approach. As for the changes with the offline installation, I do agree that it would be better to have it work on detecting an offline install using the |
This PR adds the modified theme from #143. I rebuilt the CSS against the latest SASS template from master before committing it.
(@didli didn't have this in a fork on their own GitHub, so I made a branch in mine).
Please don't merge this to master, but rather to a separate branch. That way one can choose to apply Nord (or another theme) by specifying it to the install script. (@Weilbyte you mention in #143 that it would be nice to have a more centralized repo for multiple themes, and I agree. I think multiple branches is one easy way?)
A couple of issues that remain:
REPO=b-/PVEDiscordDark TAG=nord sudo ./PVEDiscordDark.sh install
. It might not be something wrong with the installer — Maybe it's something silly with my shell. Or worse, maybe I'm getting old and forgetting syntax. But either way, there should be a more friendly way to pick a theme if there is more than one choice :)