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

Add gitea-dark theme #12448

Closed
wants to merge 12 commits into from
Closed

Add gitea-dark theme #12448

wants to merge 12 commits into from

Conversation

silverwind
Copy link
Member

@silverwind silverwind commented Aug 6, 2020

Second take on a general dark theme that is close to an inversion of the gitea theme. This is not quite ready yet but good enough for a preview.

The way it work is it hooks into webpack's output to grab index.css, remap all colors within to dark colors and prepend the result to the theme file. One benefit of this method is is that maintainers generally don't need to touch the theme as long a they are using colors already present on the page.

The hook may fail on watch mode builds after the first, I will look into possible solutions. Also, lazy-loaded chunks like dropzone are not handled yet.

image

@silverwind silverwind marked this pull request as draft August 6, 2020 21:32
@jolheiser jolheiser added type/enhancement An improvement of existing functionality topic/ui Change the appearance of the Gitea UI labels Aug 6, 2020
@silverwind silverwind mentioned this pull request Aug 7, 2020
@lafriks
Copy link
Member

lafriks commented Aug 8, 2020

Imho we should move out colors to less variables like ($text-color, $link-color, $button-background-color etc) that could be tuned than for different themes

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 8, 2020
@bagasme
Copy link
Contributor

bagasme commented Aug 8, 2020

@silverwind the message text in <div class="ui positive message" should be made white for readability, e.g.:
current:
current
white text:
white text

@lunny
Copy link
Member

lunny commented Aug 8, 2020

I like the dark theme but I think we should keep less themes to maintain easier. Currently we have two internal themes, if we merge this, we will have three. Every PR which will add a new feature(i.e. board) have to change the three themes.

Maybe we should add a theme market feature and the site https://themes.gitea.io to allow user upload and download the themes but not merge them into the main thread branches.

related #7903

@silverwind
Copy link
Member Author

silverwind commented Aug 8, 2020

we should move out colors to less variables

Less variables are only changeable at build time, not at runtime. I'd be much better to use CSS variabled (custom properties) which can be modified at runtime but I guess that could also be material of another PR.

I like the dark theme but I think we should keep less themes to maintain easier. Currently we have two internal themes, if we merge this, we will have three.

I'm not sure yet what to do about arc-green but I imagine we could eventually phase it out or move it to the automated approach with the color remapping. Maintaining these theme will be less work than it currently is where you need to copy every color-affecting selector to arc-green because as long as no new colors are introduced, the build mechanism will automatically generate the selectors for you.

Maybe we should add a theme market feature

A noble idea but I think it's currently unfeasible because manually created themes would need constant maintenance to be kept up to date with changes in the base theme. This might become feasible once we move all colors to CSS varibles, but currently it isn't.

@lafriks
Copy link
Member

lafriks commented Aug 8, 2020

@silverwind yes but this way we can build multiple themes by just specifying different variable colors. Also we can specify them to css variables to get them changeable at runtime (auto dark theme)

@silverwind
Copy link
Member Author

silverwind commented Aug 8, 2020

CSS vars can do most of the things that Less vars can do, I see no benefit in using Less vars. We can define a set of base vars in the default theme, to be overridden in each theme individually. There is one major problem which is Fomantic hardcodes all their colors so the approach can not properly work unless we fork Fomantic's CSS (which is horribly over-complicated) or generate the gitea theme using the ThemeBuilder mechanism too (I think the latter is the way to go).

@silverwind
Copy link
Member Author

@bagasme fixed that:

image

@silverwind silverwind marked this pull request as ready for review August 9, 2020 21:18
@silverwind
Copy link
Member Author

Can't see any more obvious issues, so ready for review.

@silverwind
Copy link
Member Author

Maybe should label this as a feature.

I'll do another pass on this shortly.

@codecov-commenter
Copy link

codecov-commenter commented Aug 14, 2020

Codecov Report

Merging #12448 into master will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #12448      +/-   ##
==========================================
+ Coverage   43.76%   43.77%   +0.01%     
==========================================
  Files         631      631              
  Lines       69870    69870              
==========================================
+ Hits        30580    30587       +7     
+ Misses      34338    34331       -7     
  Partials     4952     4952              
Impacted Files Coverage Δ
modules/setting/setting.go 46.44% <ø> (ø)
modules/indexer/stats/db.go 43.47% <0.00%> (-17.40%) ⬇️
modules/indexer/stats/queue.go 64.70% <0.00%> (-11.77%) ⬇️
modules/avatar/avatar.go 50.00% <0.00%> (-4.77%) ⬇️
modules/git/utils.go 73.77% <0.00%> (-3.28%) ⬇️
modules/queue/unique_queue_disk_channel.go 53.84% <0.00%> (-1.54%) ⬇️
modules/queue/workerpool.go 58.77% <0.00%> (-1.23%) ⬇️
modules/git/repo.go 49.23% <0.00%> (-0.51%) ⬇️
models/error.go 36.28% <0.00%> (+0.86%) ⬆️
models/notification.go 67.04% <0.00%> (+0.91%) ⬆️
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eb60a5d...02c870b. Read the comment docs.

@silverwind
Copy link
Member Author

Theme is now completely running on CSS vars.

@silverwind
Copy link
Member Author

silverwind commented Aug 24, 2020

Will do a few more tweaks later. If anyone has feedback on the CSS variable approach, it'd be welcome. The variable naming is certainly not ideal and I'm contemplating using shorter names like black-alpha-20. Now I think best practice is to have semantic names only but I have a hard time semantically naming 15+ shades of gray.

I imagine we can switch the base theme to the same approach using remap-css later. I recently introduced a new keep option which would allow to completely replace index.css with the remapped variant which means we can introduce CSS vars even for fomantic's styles. So the gitea theme would have all its colors in variables and the themes override those which makes the theme files emit minimal CSS. Maybe I'll add the approach here but it's a bit of work to identify all the colors first.

@techknowlogick techknowlogick added this to the 1.14.0 milestone Sep 5, 2020
@stale
Copy link

stale bot commented Nov 9, 2020

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 months. Thank you for your contributions.

@stale stale bot added the issue/stale label Nov 9, 2020
@silverwind
Copy link
Member Author

Will probably follow up on this later once we have converted everything to CSS vars.

@silverwind silverwind closed this Nov 9, 2020
@go-gitea go-gitea locked and limited conversation to collaborators Dec 14, 2020
@silverwind silverwind deleted the dark branch October 16, 2023 21:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue/stale lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. topic/ui Change the appearance of the Gitea UI type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants