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.

Sign up for GitHub

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

Jump to bottom

Add automatic theme discovery #11939

Closed
wants to merge 2 commits into from
Closed

Add automatic theme discovery #11939

wants to merge 2 commits into from

Conversation

silverwind
Copy link
Member

@silverwind silverwind commented Jun 17, 2020 •
edited
Loading

Introduce a new special value * for ui.THEMES which will automatically discover installed themes in bindata, custom folder and regular files.

Ref: #11118

@techknowlogick techknowlogick added the topic/ui Change the appearance of the Gitea UI label Jun 17, 2020
@techknowlogick techknowlogick added this to the 1.13.0 milestone Jun 17, 2020
@techknowlogick techknowlogick added the type/enhancement An improvement of existing functionality label Jun 17, 2020
silverwind
silverwind commented Jun 17, 2020
View reviewed changes
routers/init.go Outdated Show resolved Hide resolved
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jun 17, 2020
@CirnoT
Copy link
Contributor

CirnoT commented Jun 17, 2020

Would that make it impossible to remove Gitea's original themes? Could be a big deal for customized instances.

@silverwind
Copy link
Member Author

Yes, shipped themes would be always included. While we could have an option to exclude them, I think it's not a big deal if they are there.

@CirnoT
Copy link
Contributor

CirnoT commented Jun 17, 2020 •
edited
Loading

There are many instances running around that customize their theme; in fact based on my short experience from people asking for help on Discord, it happens more often than not.

Usually their point is to add custom company branding/colors/etc, but there are also projects like https://codeberg.org/ which would have to either stop using bindata or patch against our codebase.

@silverwind
Copy link
Member Author

Customize via custom CSS or add entirely new themes? I think it's more of the former, not the latter. Could have a DISABLE_THEMES setting (default empty) if really needed, but I doubt it will be of much use.

@silverwind
Copy link
Member Author

silverwind commented Jun 17, 2020 •
edited
Loading

Codeberg does not add its own theme file (e.g. custom/public/theme-*.css), it's just adding a custom CSS file codeberg.css to overwrite some parts of the default theme so will be fine.

@silverwind
Copy link
Member Author

They might want to cruely limit users to a single, customized theme thought. I'm thinking maybe retain THEMES but default it to a special * value meaning all installed themes will be automatically discovered. Only if the user specifies the option then themes would be limited as it's currently.

@silverwind silverwind marked this pull request as draft June 18, 2020 16:42
@silverwind
Add automatic theme discovery
c4a2695 
Introduce a new special value '*' for ui.THEMES which will automatically
discover installed themes in bindata, custom folder and regular files.

Ref: go-gitea#11118
@silverwind silverwind changed the title Add theme discovery and remove THEMES setting Add automatic theme discovery Jun 19, 2020
@silverwind silverwind marked this pull request as ready for review June 19, 2020 20:13
@silverwind
Copy link
Member Author

Reworked this so it now only introduces theme discovery via * value and changes the default to this value, meaning THEMES will work like before for values other than *.

silverwind
silverwind commented Jun 19, 2020
View reviewed changes
modules/themes/themes.go

// Init initializes theme-related variables
func Init() {
if reflect.DeepEqual(setting.UI.Themes, []string{"*"}) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure if this is the best way to compare two slices 😉

silverwind
silverwind commented Jun 19, 2020
View reviewed changes
modules/themes/themes_test.go

func TestThemes(t *testing.T) {
Init()
assert.Contains(t, Themes, "gitea")
Copy link
Member Author

@silverwind silverwind Jun 19, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Would like to do a full test here but I have no idea how to properly initialize the setting module for the test. If I used models.PrepareTestEnv(t) it just segfaults during the test.

@silverwind
Copy link
Member Author

Will update to not depend on gobwas/glob, putting to draft til then.

@silverwind silverwind marked this pull request as draft June 27, 2020 21:02
@stale
Copy link

stale bot commented Aug 27, 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 Aug 27, 2020
@lunny lunny modified the milestones: 1.13.0, 1.14.0 Sep 1, 2020
@stale stale bot removed the issue/stale label Sep 1, 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
@lunny lunny added the issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented label Nov 9, 2020
@stale stale bot removed the issue/stale label Nov 9, 2020
@lunny lunny modified the milestones: 1.14.0, 1.15.0 Jan 27, 2021
@techknowlogick techknowlogick modified the milestones: 1.15.0, 1.16.0 Jun 15, 2021
@lunny lunny removed this from the 1.16.0 milestone Nov 9, 2021
@silverwind
Copy link
Member Author

Would still like to do this, but this is too outdated now and I guess I would do it once I decide to make a new dark theme :)

@silverwind silverwind closed this Apr 1, 2023
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
@wxiaoguang
Copy link
Contributor

automatic theme discovery

-> Initial support for colorblind-friendly themes #30625

@wxiaoguang wxiaoguang deleted the rmtheme branch April 21, 2024 10:23
@wxiaoguang wxiaoguang restored the rmtheme branch April 21, 2024 10:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@silverwind @CirnoT @wxiaoguang @lunny @techknowlogick @GiteaBot