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

[Bug]: NC25 not working with disabled Theming app #34338

Closed
5 of 9 tasks
Rello opened this issue Sep 30, 2022 · 12 comments · Fixed by #34609
Closed
5 of 9 tasks

[Bug]: NC25 not working with disabled Theming app #34338

Rello opened this issue Sep 30, 2022 · 12 comments · Fixed by #34609

Comments

@Rello
Copy link
Contributor

Rello commented Sep 30, 2022

⚠️ This issue respects the following points: ⚠️

  • This is a bug, not a question or a configuration/webserver/proxy issue.
  • This issue is not already reported on Github (I've searched it).
  • Nextcloud Server is up to date. See Maintenance and Release Schedule for supported versions.
  • Nextcloud Server is running on 64bit capable CPU, PHP and OS.
  • I agree to follow Nextcloud's Code of Conduct.

Bug description

when the theming app is disabled, NC is loaded without css.
Error "Could not find initial state background of theming" in browser console

Steps to reproduce

  1. Apps -> disable Theming app
  2. Refresh Page

Expected behavior

Avoid the feature to disable the Theming app, when its that important

Installation method

No response

Operating system

No response

PHP engine version

No response

Web server

No response

Database engine version

No response

Is this bug present after an update or on a fresh install?

No response

Are you using the Nextcloud Server Encryption module?

No response

What user-backends are you using?

  • Default user-backend (database)
  • LDAP/ Active Directory
  • SSO - SAML
  • Other

Configuration report

No response

List of activated Apps

Theming

Nextcloud Signing status

No response

Nextcloud Logs

No response

Additional info

No response

@Rello Rello added bug 0. Needs triage Pending check for reproducibility or if it fits our roadmap labels Sep 30, 2022
@genma
Copy link

genma commented Oct 5, 2022

I've the same issuses a written a blog post in French with some screenshot https://blog.genma.fr/?Nextcloud-25-l-application-theming-casse-l-affichage-des-CSS

@AndyScherzinger AndyScherzinger added this to the Nextcloud 25 milestone Oct 6, 2022
@AndyScherzinger
Copy link
Member

Looping in @PVince81 and @juliushaertl

@PVince81
Copy link
Member

and @Pytal

@Pytal
Copy link
Member

Pytal commented Oct 12, 2022

Some dashboard js depends on theming to adjust the header

Tracking in #33969

@nickvergessen
Copy link
Member

Since the theming app now also hosts the accessibility settings, should we force enable it?
Feels weird that an admin can decide to punish people for their accessibility lacks by disabling an app they just don't need themselves?

@szaimen szaimen added 2. developing Work in progress and removed 0. Needs triage Pending check for reproducibility or if it fits our roadmap labels Oct 14, 2022
@szaimen
Copy link
Contributor

szaimen commented Oct 14, 2022

WIP here: #34609 however I didn't test yet what happens if you disable the app and upgrade to this branch.

@kyrofa
Copy link
Member

kyrofa commented Apr 30, 2023

Hey @szaimen, the theming app is disabled by default in the snap because it adds warnings about not having imagick. Did you ever test updates when this app is disabled? Any bad side effects? Has anyone considered splitting critical functionality out of a non-critical theming app so folks who don't want imagick installed don't constantly see a warning?

@nickvergessen
Copy link
Member

it's not a warning as it's not yellow and not an error as it's not red. It's an info.
The problem is when there are no errors/warnings the headlines still say "Setup warnings", so that's why you might confuse it for that.

Also note, since Nextcloud 25 the Theming app is one of the apps that must always be enabled:
https://github.com/nextcloud/server/blob/stable25/core/shipped.json#L100
https://github.com/nextcloud/server/blob/stable26/core/shipped.json#L100
https://github.com/nextcloud/server/blob/master/core/shipped.json#L100

The reason is it was merged with the accessibility features as they shared to many settings and technics and need to work hand-in-hand, but Accessibility is not optional, it's one of the core principals of the Nextcloud project, see https://nextcloud.com/about/ (middle of the page, seems to miss anchors at the moment).
So please remove any code that would try to disable/remove/prevent theming.

@kyrofa
Copy link
Member

kyrofa commented May 1, 2023

it's not a warning as it's not yellow and not an error as it's not red. It's an info.
The problem is when there are no errors/warnings the headlines still say "Setup warnings", so that's why you might confuse it for that.

That's a bit pedantic. The people who log bugs about that don't see it that way. Fixing that messaging would be helpful.

So please remove any code that would try to disable/remove/prevent theming.

That's just signing us up for triaging more invalid bug reports. Can we please consider moving accessibility to another trimmed down app specifically for it?

@szaimen
Copy link
Contributor

szaimen commented May 1, 2023

I think the best way forward would be this: #36607

@tonyguepin
Copy link

Due to the theming app being forcibly enabled, as of 25,
we're running into problems when using our own customized theme which required the theming app to be disabled.
Our customized theme redefines some css-vars and styles , these are now being overwritten by the theming app.

Do I understand correctly that the reason for the forcibly enabled theming app is the need for "accessibility features" ?

@gioman
Copy link

gioman commented Jul 29, 2023

Due to the theming app being forcibly enabled, as of 25, we're running into problems when using our own customized theme which required the theming app to be disabled. Our customized theme redefines some css-vars and styles , these are now being overwritten by the theming app.

Do I understand correctly that the reason for the forcibly enabled theming app is the need for "accessibility features" ?

@tonyguepin similar situation here. I was trying to use just one instance of NC 27 for multiple domain names, and depending on the domain show a different theme. I spent quite a lot in reading posts and docs, and it seemed feasible as some of the configs worked, while others not

i.e.
https://help.nextcloud.com/t/howto-individual-themes-per-domain/27585

the part that isn't working is the override/injection of some CSS property, as described in this legacy page of the docs

https://docs.nextcloud.com/server/12/developer_manual/core/theming.html

and finally/sadly read this post

https://help.nextcloud.com/t/howto-individual-themes-per-domain/27585/24

So I kindly ask: now that is not possible to disable theming, how to override/inject CSS properties into custom themes ("themes" folder, using the "example" theme/folder as template for others) now that functions in defaults.php like

public function getScssVariables() {
    return [
        'color-primary' => '#745bca'
    ];
}

are ignored or simply overridden by the theming functionality?

Thanks in advance

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants