Merge pull request #4583 from nextcloud/theming-fixes-admin

Theming admin section fixes

6 files changed, 114 insertions, 131 deletions

diff --git a/apps/theming/css/settings-admin.css b/apps/theming/css/settings-admin.css
index 60b9c080ad6..8cc3f0dfe14 100644
--- a/apps/theming/css/settings-admin.css
+++ b/apps/theming/css/settings-admin.css
@@ -1,5 +1,10 @@
 #theming input {
-    width: 17em;
+    width: 230px;
+}
+
+#theming input:focus,
+#theming input:active {
+    padding-right: 30px;
 }
 
 #theming .upload-logo-field {
@@ -38,19 +43,39 @@
 
 #theming label span {
     display: inline-block;
-    min-width: 90px;
+    min-width: 120px;
     padding: 8px 0px;
+    vertical-align: top;
 }
 
-#theming .icon-upload,
 #theming .icon-loading-small {
     display: inline-flex;
     padding: 8px;
     margin: 2px 0px;
 }
 
+#theming .icon-upload {
+    padding: 8px 20px;
+    width: 20px;
+    margin: 2px 0px;
+    min-height: 32px;
+    display: inline-block;
+}
+
+#theming_settings_status {
+    height: 26px;
+    margin: 10px;
+
+}
+#theming_settings_loading.icon-loading-small {
+    float: left;
+    padding: 0;
+    margin-right: 10px;
+}
 div#theming_settings_msg {
-    margin-left: 10px;
+    margin: 8px;
+    margin-left:20px;
+    display: inline-block;
 }
 
 #theming-preview {
@@ -59,7 +84,8 @@ div#theming_settings_msg {
     background-size: cover;
     background-position: center center;
     text-align: center;
-    margin-left: 93px;
+    margin-left: 123px;
+    margin-top: 10px;
 }
 
 #theming-preview img {
diff --git a/apps/theming/js/settings-admin.js b/apps/theming/js/settings-admin.js
index 216463b15e0..802f6e05c07 100644
--- a/apps/theming/js/settings-admin.js
+++ b/apps/theming/js/settings-admin.js
@@ -19,87 +19,58 @@
  *
  */
 
-function setThemingValue(setting, value) {
+function startLoading() {
 	OC.msg.startSaving('#theming_settings_msg');
+	$('#theming_settings_loading').show();
+}
+
+function setThemingValue(setting, value) {
+	startLoading();
 	$.post(
 		OC.generateUrl('/apps/theming/ajax/updateStylesheet'), {'setting' : setting, 'value' : value}
 	).done(function(response) {
-		OC.msg.finishedSaving('#theming_settings_msg', response);
 		hideUndoButton(setting, value);
+		preview(setting, value);
 	}).fail(function(response) {
 		OC.msg.finishedSaving('#theming_settings_msg', response);
+		$('#theming_settings_loading').hide();
 	});
-	preview(setting, value);
-}
-
-function calculateLuminance(rgb) {
-	var hexValue = rgb.replace(/[^0-9A-Fa-f]/, '');
-	var r,g,b;
-	if (hexValue.length === 3) {
-		hexValue = hexValue[0] + hexValue[0] + hexValue[1] + hexValue[1] + hexValue[2] + hexValue[2];
-	}
-	if (hexValue.length !== 6) {
-		return 0;
-	}
-	r = parseInt(hexValue.substring(0,2), 16);
-	g = parseInt(hexValue.substring(2,4), 16);
-	b = parseInt(hexValue.substring(4,6), 16);
-	return (0.299*r + 0.587*g + 0.114*b)/255;
-}
-
-function generateRadioButton(color) {
-	var radioButton = '<svg xmlns="http://www.w3.org/2000/svg" height="16" width="16">' +
-		'<path d="M8 1a7 7 0 0 0-7 7 7 7 0 0 0 7 7 7 7 0 0 0 7-7 7 7 0 0 0-7-7zm0 1a6 6 0 0 1 6 6 6 6 0 0 1-6 6 6 6 0 0 1-6-6 6 6 0 0 1 6-6zm0 2a4 4 0 1 0 0 8 4 4 0 0 0 0-8z" fill="' + color + '"/></svg>';
-	return btoa(radioButton);
 }
 
 function preview(setting, value) {
-	if (setting === 'color') {
-		var headerClass = document.getElementById('header');
-		var expandDisplayNameClass = document.getElementById('expandDisplayName');
-		var headerAppName = headerClass.getElementsByClassName('header-appname')[0];
-		var textColor, icon;
-		var luminance = calculateLuminance(value);
-		var elementColor = value;
-
-		if (luminance > 0.5) {
-			textColor = "#000000";
-			icon = 'caret-dark';
-		} else {
-			textColor = "#ffffff";
-			icon = 'caret';
-		}
-		if (luminance > 0.8) {
-			elementColor = '#555555';
-		}
+	OC.msg.startAction('#theming_settings_msg', t('theming', 'Loading preview…'));
+	var stylesheetsLoaded = 2;
+	var reloadStylesheets = function(cssFile) {
+		var queryString = '?reload=' + new Date().getTime();
+		var url = OC.generateUrl(cssFile) + queryString;
+		var old = $('link[href*="' + cssFile.replace("/","\/") + '"]');
+		var stylesheet = $("<link/>", {
+			rel: "stylesheet",
+			type: "text/css",
+			href: url
+		});
+		stylesheet.load(function () {
+			$(old).remove();
+			stylesheetsLoaded--;
+			if(stylesheetsLoaded === 0) {
+				$('#theming_settings_loading').hide();
+				var response = { status: 'success', data: {message: t('theming', 'Saved')}};
+				OC.msg.finishedSaving('#theming_settings_msg', response);
+			}
+		});
+		stylesheet.appendTo("head");
+	};
 
-		headerClass.style.background = value;
-		headerClass.style.backgroundImage = '../img/logo-icon.svg';
-		expandDisplayNameClass.style.color = textColor;
-		headerAppName.style.color = textColor;
-
-		$('#previewStyles').html(
-			'#header .icon-caret { background-image: url(\'' + OC.getRootPath() + '/core/img/actions/' + icon + '.svg\') }' +
-			'input[type="checkbox"].checkbox:checked:enabled:not(.checkbox--white) + label:before {' +
-			'background-image:url(\'' + OC.getRootPath() + '/core/img/actions/checkmark-white.svg\');' +
-			'background-color: ' + elementColor + '; background-position: center center; background-size:contain;' +
-			'width:12px; height:12px; padding:0; margin:2px 6px 6px 2px; border-radius:1px;}' +
-			'input[type="radio"].radio:checked:not(.radio--white):not(:disabled) + label:before {' +
-			'background-image: url(\'data:image/svg+xml;base64,' + generateRadioButton(elementColor) + '\'); }'
-		);
-	}
+	reloadStylesheets('/css/core/server.css');
+	reloadStylesheets('/apps/theming/styles');
 
+	// Preview images
 	var timestamp = new Date().getTime();
 	if (setting === 'logoMime') {
-		var logos = document.getElementsByClassName('logo-icon');
 		var previewImageLogo = document.getElementById('theming-preview-logo');
 		if (value !== '') {
-			logos[0].style.backgroundImage = "url('" + OC.generateUrl('/apps/theming/logo') + "?v" + timestamp + "')";
-			logos[0].style.backgroundSize = "contain";
 			previewImageLogo.src = OC.generateUrl('/apps/theming/logo') + "?v" + timestamp;
 		} else {
-			logos[0].style.backgroundImage = "url('" + OC.getRootPath() + '/core/img/logo-icon.svg?v' + timestamp + "')";
-			logos[0].style.backgroundSize = "contain";
 			previewImageLogo.src = OC.getRootPath() + '/core/img/logo-icon.svg?v' + timestamp;
 		}
 	}
@@ -110,8 +81,8 @@ function preview(setting, value) {
 		} else {
 			previewImage.style.backgroundImage = "url('" + OC.getRootPath() + '/core/img/background.jpg?v' + timestamp + "')";
 		}
-
 	}
+
 	if (setting === 'name') {
 		window.document.title = t('core', 'Admin') + " - " + value;
 	}
@@ -137,8 +108,6 @@ function hideUndoButton(setting, value) {
 $(document).ready(function () {
 	$('#theming [data-toggle="tooltip"]').tooltip();
 
-	$('html > head').append($('<style type="text/css" id="previewStyles"></style>'));
-
 	$('#theming .theme-undo').each(function() {
 		var setting = $(this).data('setting');
 		var value = $('#theming-'+setting).val();
@@ -157,7 +126,7 @@ $(document).ready(function () {
 			$('.theme-undo[data-setting=logoMime]').show();
 		},
 		submit: function(e, response) {
-			OC.msg.startSaving('#theming_settings_msg');
+			startLoading();
 			$('label#uploadlogo').removeClass('icon-upload').addClass('icon-loading-small');
 		},
 		fail: function (e, response){
@@ -175,7 +144,7 @@ $(document).ready(function () {
 			$('.theme-undo[data-setting=backgroundMime]').show();
 		},
 		submit: function(e, response) {
-			OC.msg.startSaving('#theming_settings_msg');
+			startLoading();
 			$('label#upload-login-background').removeClass('icon-upload').addClass('icon-loading-small');
 		},
 		fail: function (e, response){
@@ -223,7 +192,7 @@ $(document).ready(function () {
 
 	$('.theme-undo').click(function (e) {
 		var setting = $(this).data('setting');
-		OC.msg.startSaving('#theming_settings_msg');
+		startLoading();
 		$('.theme-undo[data-setting=' + setting + ']').hide();
 		$.post(
 			OC.generateUrl('/apps/theming/ajax/undoChanges'), {'setting' : setting}
@@ -236,9 +205,7 @@ $(document).ready(function () {
 				var input = document.getElementById('theming-'+setting);
 				input.value = response.data.value;
 			}
-
 			preview(setting, response.data.value);
-			OC.msg.finishedSaving('#theming_settings_msg', response);
 		});
 	});
 });
diff --git a/apps/theming/lib/Controller/ThemingController.php b/apps/theming/lib/Controller/ThemingController.php
index b2d603bb68a..cffc628e9b9 100644
--- a/apps/theming/lib/Controller/ThemingController.php
+++ b/apps/theming/lib/Controller/ThemingController.php
@@ -163,6 +163,10 @@ class ThemingController extends Controller {
 		}
 
 		$this->themingDefaults->set($setting, $value);
+
+		// reprocess server scss for preview
+		$cssCached = $this->scssCacher->process(\OC::$SERVERROOT, '/core/css/server.scss', 'core');
+
 		return new DataResponse(
 			[
 				'data' =>
@@ -259,6 +263,8 @@ class ThemingController extends Controller {
 	 */
 	public function undo($setting) {
 		$value = $this->themingDefaults->undo($setting);
+		// reprocess server scss for preview
+		$cssCached = $this->scssCacher->process(\OC::$SERVERROOT, '/core/css/server.scss', 'core');
 		return new DataResponse(
 			[
 				'data' =>
diff --git a/apps/theming/lib/ThemingDefaults.php b/apps/theming/lib/ThemingDefaults.php
index 85ad11943dd..2935355f26d 100644
--- a/apps/theming/lib/ThemingDefaults.php
+++ b/apps/theming/lib/ThemingDefaults.php
@@ -66,11 +66,11 @@ class ThemingDefaults extends \OC_Defaults {
 	public function __construct(IConfig $config,
 								IL10N $l,
 								IURLGenerator $urlGenerator,
-								\OC_Defaults $defaults,
 								IAppData $appData,
 								ICacheFactory $cacheFactory,
 								Util $util
 	) {
+		parent::__construct();
 		$this->config = $config;
 		$this->l = $l;
 		$this->urlGenerator = $urlGenerator;
@@ -78,10 +78,10 @@ class ThemingDefaults extends \OC_Defaults {
 		$this->cacheFactory = $cacheFactory;
 		$this->util = $util;
 
-		$this->name = $defaults->getName();
-		$this->url = $defaults->getBaseUrl();
-		$this->slogan = $defaults->getSlogan();
-		$this->color = $defaults->getColorPrimary();
+		$this->name = parent::getName();
+		$this->url = parent::getBaseUrl();
+		$this->slogan = parent::getSlogan();
+		$this->color = parent::getColorPrimary();
 	}
 
 	public function getName() {
diff --git a/apps/theming/templates/settings-admin.php b/apps/theming/templates/settings-admin.php
index f7c8f591fc7..858329eca48 100644
--- a/apps/theming/templates/settings-admin.php
+++ b/apps/theming/templates/settings-admin.php
@@ -29,7 +29,10 @@ style('theming', 'settings-admin');
 <div id="theming" class="section">
 	<h2 class="inlineblock"><?php p($l->t('Theming')); ?></h2>
         <p class="settings-hint"><?php p($l->t('Theming makes it possible to easily customize the look and feel of your instance and supported clients. This will be visible for all users.')); ?></p>
-		<div id="theming_settings_msg" class="msg success inlineblock" style="display: none;">Saved</div>
+		<div id="theming_settings_status">
+			<div id="theming_settings_loading" class="icon-loading-small" style="display: none;"></div>
+			<span id="theming_settings_msg" class="msg success" style="display: none;">Saved</span>
+		</div>
 	<?php if ($_['themable'] === false) { ?>
 	<p>
 		<?php p($_['errorMessage']) ?>
diff --git a/apps/theming/tests/ThemingDefaultsTest.php b/apps/theming/tests/ThemingDefaultsTest.php
index f8ecc78d8ba..736eeb3afc3 100644
--- a/apps/theming/tests/ThemingDefaultsTest.php
+++ b/apps/theming/tests/ThemingDefaultsTest.php
@@ -65,25 +65,7 @@ class ThemingDefaultsTest extends TestCase {
 		$this->cacheFactory = $this->createMock(ICacheFactory::class);
 		$this->cache = $this->createMock(ICache::class);
 		$this->util = $this->createMock(Util::class);
-		$this->defaults = $this->getMockBuilder(\OC_Defaults::class)
-			->disableOriginalConstructor()
-			->getMock();
-		$this->defaults
-			->expects($this->at(0))
-			->method('getName')
-			->willReturn('Nextcloud');
-		$this->defaults
-			->expects($this->at(1))
-			->method('getBaseUrl')
-			->willReturn('https://nextcloud.com/');
-		$this->defaults
-			->expects($this->at(2))
-			->method('getSlogan')
-			->willReturn('Safe Data');
-		$this->defaults
-			->expects($this->at(3))
-			->method('getColorPrimary')
-			->willReturn('#000');
+		$this->defaults = new \OC_Defaults();
 		$this->cacheFactory
 			->expects($this->any())
 			->method('create')
@@ -93,7 +75,6 @@ class ThemingDefaultsTest extends TestCase {
 			$this->config,
 			$this->l10n,
 			$this->urlGenerator,
-			$this->defaults,
 			$this->appData,
 			$this->cacheFactory,
 			$this->util
@@ -185,17 +166,17 @@ class ThemingDefaultsTest extends TestCase {
 		$this->config
 			->expects($this->once())
 			->method('getAppValue')
-			->with('theming', 'url', 'https://nextcloud.com/')
-			->willReturn('https://nextcloud.com/');
+			->with('theming', 'url', $this->defaults->getBaseUrl())
+			->willReturn($this->defaults->getBaseUrl());
 
-		$this->assertEquals('https://nextcloud.com/', $this->template->getBaseUrl());
+		$this->assertEquals($this->defaults->getBaseUrl(), $this->template->getBaseUrl());
 	}
 
 	public function testGetBaseUrlWithCustom() {
 		$this->config
 			->expects($this->once())
 			->method('getAppValue')
-			->with('theming', 'url', 'https://nextcloud.com/')
+			->with('theming', 'url', $this->defaults->getBaseUrl())
 			->willReturn('https://example.com/');
 
 		$this->assertEquals('https://example.com/', $this->template->getBaseUrl());
@@ -205,17 +186,17 @@ class ThemingDefaultsTest extends TestCase {
 		$this->config
 			->expects($this->once())
 			->method('getAppValue')
-			->with('theming', 'slogan', 'Safe Data')
-			->willReturn('Safe Data');
+			->with('theming', 'slogan', $this->defaults->getSlogan())
+			->willReturn($this->defaults->getSlogan());
 
-		$this->assertEquals('Safe Data', $this->template->getSlogan());
+		$this->assertEquals($this->defaults->getSlogan(), $this->template->getSlogan());
 	}
 
 	public function testGetSloganWithCustom() {
 		$this->config
 			->expects($this->once())
 			->method('getAppValue')
-			->with('theming', 'slogan', 'Safe Data')
+			->with('theming', 'slogan', $this->defaults->getSlogan())
 			->willReturn('My custom Slogan');
 
 		$this->assertEquals('My custom Slogan', $this->template->getSlogan());
@@ -226,9 +207,9 @@ class ThemingDefaultsTest extends TestCase {
 			->expects($this->exactly(3))
 			->method('getAppValue')
 			->willReturnMap([
-				['theming', 'url', 'https://nextcloud.com/', 'url'],
+				['theming', 'url', $this->defaults->getBaseUrl(), 'url'],
 				['theming', 'name', 'Nextcloud', 'Name'],
-				['theming', 'slogan', 'Safe Data', 'Slogan'],
+				['theming', 'slogan', $this->defaults->getSlogan(), 'Slogan'],
 			]);
 
 		$this->assertEquals('<a href="url" target="_blank" rel="noreferrer">Name</a> – Slogan', $this->template->getShortFooter());
@@ -239,9 +220,9 @@ class ThemingDefaultsTest extends TestCase {
 			->expects($this->exactly(3))
 			->method('getAppValue')
 			->willReturnMap([
-				['theming', 'url', 'https://nextcloud.com/', 'url'],
+				['theming', 'url', $this->defaults->getBaseUrl(), 'url'],
 				['theming', 'name', 'Nextcloud', 'Name'],
-				['theming', 'slogan', 'Safe Data', ''],
+				['theming', 'slogan', $this->defaults->getSlogan(), ''],
 			]);
 
 		$this->assertEquals('<a href="url" target="_blank" rel="noreferrer">Name</a>', $this->template->getShortFooter());
@@ -251,17 +232,17 @@ class ThemingDefaultsTest extends TestCase {
 		$this->config
 			->expects($this->once())
 			->method('getAppValue')
-			->with('theming', 'color', '#000')
-			->willReturn('#000');
+			->with('theming', 'color', $this->defaults->getColorPrimary())
+			->willReturn($this->defaults->getColorPrimary());
 
-		$this->assertEquals('#000', $this->template->getColorPrimary());
+		$this->assertEquals($this->defaults->getColorPrimary(), $this->template->getColorPrimary());
 	}
 
 	public function testgetColorPrimaryWithCustom() {
 		$this->config
 			->expects($this->once())
 			->method('getAppValue')
-			->with('theming', 'color', '#000')
+			->with('theming', 'color', $this->defaults->getColorPrimary())
 			->willReturn('#fff');
 
 		$this->assertEquals('#fff', $this->template->getColorPrimary());
@@ -328,10 +309,10 @@ class ThemingDefaultsTest extends TestCase {
 		$this->config
 			->expects($this->at(3))
 			->method('getAppValue')
-			->with('theming', 'url', 'https://nextcloud.com/')
-			->willReturn('https://nextcloud.com/');
+			->with('theming', 'url', $this->defaults->getBaseUrl())
+			->willReturn($this->defaults->getBaseUrl());
 
-		$this->assertSame('https://nextcloud.com/', $this->template->undo('url'));
+		$this->assertSame($this->defaults->getBaseUrl(), $this->template->undo('url'));
 	}
 
 	public function testUndoSlogan() {
@@ -351,10 +332,10 @@ class ThemingDefaultsTest extends TestCase {
 		$this->config
 			->expects($this->at(3))
 			->method('getAppValue')
-			->with('theming', 'slogan', 'Safe Data')
-			->willReturn('Safe Data');
+			->with('theming', 'slogan', $this->defaults->getSlogan())
+			->willReturn($this->defaults->getSlogan());
 
-		$this->assertSame('Safe Data', $this->template->undo('slogan'));
+		$this->assertSame($this->defaults->getSlogan(), $this->template->undo('slogan'));
 	}
 
 	public function testUndoColor() {
@@ -374,10 +355,10 @@ class ThemingDefaultsTest extends TestCase {
 		$this->config
 			->expects($this->at(3))
 			->method('getAppValue')
-			->with('theming', 'color', '#000')
-			->willReturn('#000');
+			->with('theming', 'color', $this->defaults->getColorPrimary())
+			->willReturn($this->defaults->getColorPrimary());
 
-		$this->assertSame('#000', $this->template->undo('color'));
+		$this->assertSame($this->defaults->getColorPrimary(), $this->template->undo('color'));
 	}
 
 	public function testUndoDefaultAction() {
@@ -502,11 +483,11 @@ class ThemingDefaultsTest extends TestCase {
 		$this->config->expects($this->at(1))->method('getAppValue')->with('theming', 'logoMime', false)->willReturn('jpeg');
 		$this->config->expects($this->at(2))->method('getAppValue')->with('theming', 'cachebuster', '0')->willReturn('0');
 		$this->config->expects($this->at(3))->method('getAppValue')->with('theming', 'backgroundMime', false)->willReturn('jpeg');
-		$this->config->expects($this->at(4))->method('getAppValue')->with('theming', 'color', null)->willReturn('#000000');
-		$this->config->expects($this->at(5))->method('getAppValue')->with('theming', 'color', '#000')->willReturn('#000000');
-		$this->config->expects($this->at(6))->method('getAppValue')->with('theming', 'color', '#000')->willReturn('#000000');
+		$this->config->expects($this->at(4))->method('getAppValue')->with('theming', 'color', null)->willReturn($this->defaults->getColorPrimary());
+		$this->config->expects($this->at(5))->method('getAppValue')->with('theming', 'color', $this->defaults->getColorPrimary())->willReturn($this->defaults->getColorPrimary());
+		$this->config->expects($this->at(6))->method('getAppValue')->with('theming', 'color', $this->defaults->getColorPrimary())->willReturn($this->defaults->getColorPrimary());
 
-		$this->util->expects($this->any())->method('invertTextColor')->with('#000000')->willReturn(false);
+		$this->util->expects($this->any())->method('invertTextColor')->with($this->defaults->getColorPrimary())->willReturn(false);
 		$this->cache->expects($this->once())->method('get')->with('getScssVariables')->willReturn(null);
 		$folder = $this->createMock(ISimpleFolder::class);
 		$file = $this->createMock(ISimpleFile::class);
@@ -532,7 +513,7 @@ class ThemingDefaultsTest extends TestCase {
 			'theming-cachebuster' => '\'0\'',
 			'image-logo' => "'absolute-custom-logo?v=0'",
 			'image-login-background' => "'absolute-custom-background'",
-			'color-primary' => '#000000',
+			'color-primary' => $this->defaults->getColorPrimary(),
 			'color-primary-text' => '#ffffff'
 
 		];