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

Apps files from theme are being ignored #5554

Closed
rebeccaformfx opened this issue Jun 29, 2017 · 8 comments
Closed

Apps files from theme are being ignored #5554

rebeccaformfx opened this issue Jun 29, 2017 · 8 comments
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap enhancement feature: theming

Comments

@rebeccaformfx
Copy link

Steps to reproduce

  1. Employ custom theme
  2. Attempt to override one of the enabled apps on my NextCloud install (breadcrumb.js, for example) by creating {{themename}}>apps>files>js>breadcrumb.js to my theme
  3. (I just added a console.log() command to print to console)

Expected behaviour

Tell us what should happen

breadcrumb.js file in my theme should override the root file

Actual behaviour

Tell us what happens instead

NextCloud ignores my theme file

Server configuration

Operating system:

Web server:

Database:

PHP version:

Nextcloud version: (see Nextcloud admin page)

Updated from an older Nextcloud/ownCloud or fresh install:

Where did you install Nextcloud from:

Signing status:

Signing status
Login as admin user into your Nextcloud and access 
http://example.com/index.php/settings/integrity/failed 
paste the results here.

List of activated apps:

App list
If you have access to your command line run e.g.:
sudo -u www-data php occ app:list
from within your Nextcloud installation folder

Nextcloud configuration:

Config report
If you have access to your command line run e.g.:
sudo -u www-data php occ config:list system
from within your Nextcloud installation folder

or 

Insert your config.php content here. 
Make sure to remove all sensitive content such as passwords. (e.g. database password, passwordsalt, secret, smtp password, …)

Are you using external storage, if yes which one: local/smb/sftp/...

Are you using encryption: yes/no

Are you using an external user-backend, if yes which one: LDAP/ActiveDirectory/Webdav/...

LDAP configuration (delete this part if not used)

LDAP config
With access to your command line run e.g.:
sudo -u www-data php occ ldap:show-config
from within your Nextcloud installation folder

Without access to your command line download the data/owncloud.db to your local
computer or access your SQL server remotely and run the select query:
SELECT * FROM `oc_appconfig` WHERE `appid` = 'user_ldap';


Eventually replace sensitive data as the name/IP-address of your LDAP server or groups.

Client configuration

Browser:

Operating system:

Logs

Web server error log

Web server error log
Insert your webserver log here

Nextcloud log (data/nextcloud.log)

Nextcloud log
Insert your Nextcloud log here

Browser log

Browser log
Insert your browser log here, this could for example include:

a) The javascript console log
b) The network log
c) ...
@rebeccaformfx
Copy link
Author

I've seen a number of other bugs on OwnCloud's github about this problem. Hopefully NextCloud can address this where OwnCloud has failed. owncloud/core#8626

@MorrisJobke
Copy link
Member

In general I don't like to allow overwriting of our code. This is basically patching our code which could cause severe problems.

cc @LukasReschke @nickvergessen @rullzer @nextcloud/theming

I would vote for not allow such behaviour. If you want to mess with that code: apply patches - yes it is bad, but randomly replacing files from within a theme is not better at all.

@MorrisJobke MorrisJobke added the 0. Needs triage Pending check for reproducibility or if it fits our roadmap label Sep 26, 2017
@LukasReschke
Copy link
Member

I would vote for not allow such behaviour. If you want to mess with that code: apply patches - yes it is bad, but randomly replacing files from within a theme is not better at all.

👍

@juliushaertl
Copy link
Member

Makes sense to remove support for that. But it should then also be removed from the documentation.

@rebeccaformfx
Copy link
Author

rebeccaformfx commented Sep 26, 2017

Understood as to why this is on principal a bad idea. That being said, I would suggest to please limit the amount of times that inline CSS styles and image paths are created from JS files within apps. It makes it very hard to make a thorough visual theme since these paths are un-overridable without changing the JS file or creating some messy JS on my side to constantly check for and replace these assets

@rebeccaformfx
Copy link
Author

rebeccaformfx commented Sep 26, 2017

Example being the breadcrumbs SVG home icon asset. Custom CSS does not affect this asset since it is an img and I cannot change the written path in the breadcrumbs.js app file to write it to the screen the way I want it in my theme, so i have to run a setInterval function to constantly swap the image source. Really messy

@MorrisJobke
Copy link
Member

Example being the breadcrumbs SVG home icon asset. Custom CSS does not affect this asset since it is an img and I cannot change the written path in the breadcrumbs.js app file to write it to the screen the way I want it in my theme, so i have to run a setInterval function to constantly swap the image source. Really messy

This is then a really specific problem and we need to work on this. Could you open an ticket, that describes this and then we can look into how to make theming this icon easier.

@rebeccaformfx
Copy link
Author

rebeccaformfx commented Sep 26, 2017

There are other areas of the app that do this. For example, the colors of the values under the Size and Modified columns in the Files app are written inline through the Files app JS files and cannot be overwritten by CSS. Instead of inline CSS which always takes precendence you should add a class that can be overridden through CSS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap enhancement feature: theming
Projects
None yet
Development

No branches or pull requests

5 participants