Implement a dark theme #5964
Implement a dark theme #5964
Conversation
|
@mrahhal Thanks for this PR. It's always great seeing such contributions from the community. As you suspected, it'll take us time to fully review and potentially merge this PR. PRs that affect UI/UX require a more extensive review. I'm fairly certain we'll get feedback from our internal UX team about some changes, though I feel they'd be in favor of such functionality in general. I hope you can understand why this is going to take longer for a full review, and I appreciate your patience in the matter. |
|
Yeah I understand no worries. If at the time the team thinks this is worth it, I'll be ready to cooperate and get this PR into a good shape. |
|
Let's solve these conflicts and merge them, guys! This will help us a lot given a new face to this old dude. |
|
any news about that? my eyes are begging hahaha |
|
I'm ready whenever to resolve conflicts and get this into good shape, but it's hanging on the maintainers. Can't work on this more without it getting the "yes". |
|
Nice work, i work with cherwell and this would help a lot on the eyes. Thanks for the work you do. |
|
Fixes #6448 |
Any update on the review? |
|
@mrahhal Thanks for the PR! Unfortunately, we are unable to accept and merge this PR. I think it is a good idea though, and I believe that a custom plugin could be used to replicate your efforts that your team (and others) can use. |
|
WTF ? This is the kind of answer that a proprietary support team would formulate. 😣 Where's the justification ? |
Based -- on -- what? |
|
Based on fuck knows what. Unfortunately, some developers absolutely loathe improvements to their projects. |
|
@Atulin that kind of language is uncalled for. The decision was mine when I was still working on the project. Even though I'm not anymore, I'm not going to go into the details as to why not, even though they were justified (and still are). An alternative suggestion was given (and there's nothing wrong with it), and with time, the decision will likely to be clear. |
|
>they were justified and still are Aight |
|
Yes, you're not owed an explanation. |
|
In that case, you might as well make the project proprietary. |
|
@tim-lai please consider this PR. There is literally no explanation! |
|
Generated a simple dark theme using Dark Reader, if anyone are interessed: https://github.com/gumbarros/swagger-dark-theme |
Thanks for letting the community know, could you please elaborate on the reasons why? While I’m aware of @webron's later comment
and agree with it - after all, we all can decide what to do with our own time - letting the community know why this could (or still can't) be merged will enable other potential contributors to decide if they should invest time in this issue or not. Without any explanation, this is not possible. For example, if it is a general problem with the amount of maintenance required (which is a valid decision that other projects have made in the past), this would let everyone know to create a community-maintained dark mode extension. If it is however any other issue, e.g. the style of the code, this would serve as encouragement for others who want this feature to tackle it. It would be very helpful for everyone here and everyone eventually finding this PR to know why it was rejected, so would you please be able to provide us with additional context for this decision? |
|
Theming can be a very subjective experience, and what works for some may not work for others. There are many users with different SwaggerUI needs, which expands beyond branding and theming. As such, users can customize their experience by leveraging SwaggerUI's plugin system. Thanks all for the passion, so let's all keep it friendly and nice here, even if we disagree with certain decisions or how they may have come across. |
|
@tim-lai Thank you for clarifying! |
|
Well, according to this logic, the Swagger UI should be fully unstyled (CSS-less) when running without plugin, so that even the current base theme would need to be added as a plugin. |
|
@KaKi87 If that's what you take from it, you're welcome to apply this to any project of your own. But the current theme is what the swagger UI includes, which is the classic 80% solution: Sufficient for most cases. Maintaining two themes that should only differ by color scheme is an overhead that not every project can or wants to afford. |
|
Well, glad we at least got an explanation instead of "get bent lmao" |
I won't follow this logic, so I won't do that.
If that was true, then the Swagger UI would be dark-only, considering the following stats : |
|
@KaKi87 The statistics you posted are not relevant to swagger as they are about completely different projects/software. The maintainers have pointed out why this will not be happening. I can understand why people are unhappy about it, but unless there are significant new arguments here, I do not see why they should change their mind. This will be my last comment on this issue as there is nothing further to discuss here. Anyone who disagrees with a decision someone makes about how they use their free time (or how a company decides to use the time they pay their employees for) on an open source project is welcome to fork it. That being said, a good day to everyone here and thanks to the maintainers for maintaining this project. |
I have pointed out why that logic is invalid.
Actually, dev-specific surveys show even higher dark mode usage stats.
Why would new arguments be needed ? It's simple : Swagger UI users are the kind of people who mostly prefer dark UIs. And there's no point in doing open source if you don't want to listen to users. |
Why not integrate a widely-used upstream theme? Other apps simply use one of the major dark themes: "dracula", "nord dark", etc. These are very popular and would satisfy almost every dark mode user. And since the theme would be based on a third-party theme, you wouldn't have to entertain "please change this button or colour" requests - you could tell people to create theme plugins if they want to tweak it. It's a compromise, and would be easier to maintain long-term, as the maintenance is done by the upstream project. (@mrahhal No offense to you buddy, I personally like your theme... I would use it!) |
|
None taken. I wasn't planning to comment on this since now I just resort to using Dark Reader as the easy way out (won't beat a well designed dark theme, but good enough). I still can't take a single look at the light theme, and a lot of people just won't be using Dark Reader or user styles, so I think this issue still stands. Personally though, I understand how much of a maintanance effort merging this PR would require because of how swagger-ui was originally written (I made this clear in my first comment), which is why I didn't comment after. I consider this a good enough reason to close this, but I'm confused why none of the maintainers tried to explain the right reasons (this reason). Saying users have different needs isn't convincing in the least especially when the majority prefers dark themes nowadays. In any case, thank you for taking a look at this. For anyone else still struggling, try and use Dark Reader (hint, you can make it so that it's active only on websites you define), it'll help with those remaining apps that don't offer a theme that doesn't strain the eyes. |
|
Nope, that isn't easy work (the work in this PR was far from complete) and honestly not worth it if it's not going to be part of the project proper. |
The I never thought that the desire for an optional dark theme would generate such a response and attitude from the developers/contributors. |
|
The problem with just using Dark Reader directly is that you can't set it to run on a per-path basis, only per-domain. For example, Gitea supports dark mode natively, so I don't want Dark Reader messing with the theme, but the Swagger API docs don't. For anyone interested, I've made a userscript that automatically applies @gumbarros's stylesheet on any Swagger API documentation page (checking for the presence of the |
If the application with Swagger source code is yours, you can also download the .css and inject it like I said on README. |
Yep, I'll do that as well, just thought I'd make a more universal userscript for sites I don't control. |
Doesn't work there. |
|
@KaKi87 Works for me with Violentmonkey in Chrome. What browser and userscript manager are you using? |
|
Same extension on Chromium-browser browsers even if not Google Chrome itself, so pretty much the same. However, I did find out the issue : why did you put a If the script runs too soon, the DOM isn't ready yet. Removing it (i.e. using the default |
|
@KaKi87 Fixed, I was just concerned about potential white flashes but it doesn't seem to flash even if I remove that value. |
|
@tim-lai why? what are the requirements? optional configuration via UI? |
|
This should be on the top of the priority list. |
Obrigrado ajudou de mais, meus olhos já estavam rachados de tanto ver cor branca na tela. |
Description
Disclaimer: I did read the contribution guidelines, and even though I understand this might not be of any priority, I decided to have a go at it anyway. We use swagger extensively in our projects and have always thought a dark mode would be easier on the eyes for a lot of people.
This PR implements a theme system using css-theming. I do admit that the style changes are a bit shoddy, but a part of that is that swagger-ui's styles aren't made to work with themes in mind (which is of course normal), so I decided to just do a fast job instead of changing things radically to see if it could be accomplished easily. If we decide to go forward with this feature, things can be resolved in a better way.
Of course, this is a work in progress awaiting for feedback.
Notes:
Motivation and Context
Closes #5327
How Has This Been Tested?
I've used
npm run devto test the changes manually. The code change is extremely minimal.I've run all existing tests and made sure they worked.
Screenshots:
Checklist
My PR contains...
src/is unmodified: changes to documentation, CI, metadata, etc.)package.json)My changes...
Documentation
Automated tests