Skip to content
This repository has been archived by the owner on Nov 1, 2020. It is now read-only.

Clean all superposed white backgrounds #164

Merged
merged 1 commit into from
Feb 10, 2017
Merged

Clean all superposed white backgrounds #164

merged 1 commit into from
Feb 10, 2017

Conversation

skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Oct 9, 2016

@skjnldsv skjnldsv added blocked by core design Related to the design 2. developing Work in progress labels Oct 9, 2016
@skjnldsv skjnldsv self-assigned this Oct 9, 2016
@oparoz
Copy link
Member

oparoz commented Oct 9, 2016

Thank you for that. Ping me when it's merged in core. And btw, all commits now need to be signed-off before they can be merged.

@skjnldsv
Copy link
Member Author

Ah snap! I forgot! Thanks!

@codecov-io
Copy link

Current coverage is 99.75% (diff: 100%)

Merging #164 into master will not change coverage

@@             master       #164   diff @@
==========================================
  Files            39         39          
  Lines          1201       1201          
  Methods         170        170          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits           1198       1198          
  Misses            3          3          
  Partials          0          0          

Powered by Codecov. Last update ec0d5a0...39cd9b5

@skjnldsv
Copy link
Member Author

We can merge this one. Still a simple cleanup.
The server one will follow after the scss pr and some scss cleanup.
Right now we still don't need a white background here. :)

@oparoz

@skjnldsv skjnldsv added 3. to review Waiting for reviews and removed 2. developing Work in progress blocked by core labels Nov 30, 2016
@oparoz
Copy link
Member

oparoz commented Dec 4, 2016

Thanks for the note.
I (or someone else) need to test this with the background switcher to make sure it still works when we need to select a white background. Once confirmed, I'll merge it.

@oparoz
Copy link
Member

oparoz commented Feb 10, 2017

I don't think it's going to fix anything as the white background is added via JS afterwards, but there is no harm in removing it.

@oparoz oparoz merged commit 5f6fa23 into master Feb 10, 2017
@oparoz oparoz deleted the white-bg-cleanup branch February 10, 2017 08:38
@skjnldsv
Copy link
Member Author

^^

@skjnldsv
Copy link
Member Author

skjnldsv commented Oct 14, 2017

@oparoz Can you explain to me why there is a white background added on image opening?
I don't fully get the point of

gallery/js/galleryview.js

Lines 493 to 501 in 484b964

_setBackgroundColour: function () {
var wrapper = $('#content-wrapper');
var albumDesign = Gallery.config.albumDesign;
if (!$.isEmptyObject(albumDesign) && albumDesign.background) {
wrapper.css('background-color', albumDesign.background);
} else {
wrapper.css('background-color', '#fff');
}
},

And more importantly, the Gallery.config.albumDesign is only set to null once in the js file, so I'm not sure this whole function is required. :) Can you check?

Gallery.config.albumDesign = null;

It's the only thing left here that block the dark theme integration ;)

@oparoz
Copy link
Member

oparoz commented Oct 14, 2017

The background is set via the gallery.cnf config files.
https://github.com/nextcloud/gallery/wiki/Gallery-configuration#album-configuration

White is the standard background colour. It could come from the Nextcloud theme is there is JS API for it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
3. to review Waiting for reviews design Related to the design
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants