Allows adding of hex color to the theme-color

Fixes #7158. Adds a # on the color if missing.

Increased maxlength, added hash:true for jscolor, and adding a # if not present on the change event.

Since the input element now allows a hex code, changed values to hexcode.

In addition, added a function to get RGB array from hex or rgb values. Calling it in both methods and using it to perform comparison.

Also changed the way we were determining whether the jscolor component had loaded.

Changed the control to use data-jscolor rather than defining opts in the class.

Signed-off-by: Abijeet <abijeetpatro@gmail.com>

apps/theming/js/settings-admin.js

@@ -206,7 +206,11 @@ $(document).ready(function () {
 	});
 
 	$('#theming-color').change(function (e) {
-		setThemingValue('color', '#' + $(this).val());
+		var color = $(this).val();
+		if (color.indexOf('#') !== 0) {
+			color = '#' + color;
+		}
+		setThemingValue('color', color);
 	});
 
 	$('.theme-undo').click(function (e) {

apps/theming/templates/settings-admin.php

@@ -62,7 +62,7 @@ style('theming', 'settings-admin');
 	<div>
 		<label>
 			<span><?php p($l->t('Color')) ?></span>
-			<input id="theming-color" type="text" class="jscolor" maxlength="6" value="<?php p($_['color']) ?>" />
+			<input id="theming-color" type="text" class="jscolor" data-jscolor="{hash:true}" maxlength="7" value="<?php p($_['color']) ?>" />
 			<div data-setting="color" data-toggle="tooltip" data-original-title="<?php p($l->t('Reset to default')); ?>" class="theme-undo icon icon-history"></div>
 		</label>
 	</div>

tests/acceptance/features/app-theming.feature

@@ -5,19 +5,19 @@ Feature: app-theming
     And I visit the settings page
     And I open the "Theming" section
     And I see that the color selector in the Theming app has loaded
-    And I see that the header color is "0082C9"
-    When I set the "Color" parameter in the Theming app to "C9C9C9"
+    And I see that the header color is "#0082C9"
+    When I set the "Color" parameter in the Theming app to "#C9C9C9"
     Then I see that the parameters in the Theming app are eventually saved
-    And I see that the header color is "C9C9C9"
+    And I see that the header color is "#C9C9C9"
 
   Scenario: resetting the color updates the header color
     Given I am logged in as the admin
     And I visit the settings page
     And I open the "Theming" section
     And I see that the color selector in the Theming app has loaded
-    And I set the "Color" parameter in the Theming app to "C9C9C9"
+    And I set the "Color" parameter in the Theming app to "#C9C9C9"
     And I see that the parameters in the Theming app are eventually saved
-    And I see that the header color is "C9C9C9"
+    And I see that the header color is "#C9C9C9"
     When I reset the "Color" parameter in the Theming app to its default value
     Then I see that the parameters in the Theming app are eventually saved
-    And I see that the header color is "0082C9"
+    And I see that the header color is "#0082C9"

tests/acceptance/features/bootstrap/ThemingAppContext.php

@@ -84,24 +84,23 @@ class ThemingAppContext implements Context, ActorAwareInterface {
 	 * @Then I see that the color selector in the Theming app has loaded
 	 */
 	public function iSeeThatTheColorSelectorInTheThemingAppHasLoaded() {
-		// When the color selector is loaded it removes the leading '#' from the
-		// value property of the input field object it is linked to, and changes
-		// the background color of the input field to that value. The only way
-		// to know that the color selector has loaded is to look for any of
-		// those changes.
+		// Checking if the color selector has loaded by getting the background color
+		// of the input element. If the value present in the element matches the
+		// background of the input element, it means the color element has been
+		// initialized.
 
 		PHPUnit_Framework_Assert::assertTrue($this->actor->find(self::inputFieldFor("Color"), 10)->isVisible());
 
 		$actor = $this->actor;
 
 		$colorSelectorLoadedCallback = function() use($actor) {
-			$colorSelectorValue = $actor->getSession()->evaluateScript("return $('#theming-color')[0].value;");
-
-			if ($colorSelectorValue[0] === '#') {
-				return false;
+			$colorSelectorValue = $this->getRGBArray($actor->getSession()->evaluateScript("return $('#theming-color')[0].value;"));
+			$inputBgColor = $this->getRGBArray($actor->getSession()->evaluateScript("return $('#theming-color').css('background-color');"));
+			if ($colorSelectorValue == $inputBgColor) {
+			    return true;
 			}
 
-			return true;
+			return false;
 		};
 
 		if (!Utils::waitFor($colorSelectorLoadedCallback, $timeout = 10 * $this->actor->getFindTimeoutMultiplier(), $timeoutStep = 1)) {
@@ -109,24 +108,29 @@ class ThemingAppContext implements Context, ActorAwareInterface {
 		}
 	}
 
+	private function getRGBArray ($color) {
+	    if (preg_match("/rgb\(\s*(\d+),\s*(\d+),\s*(\d+)\)/", $color, $matches)) {
+		// Already an RGB (R, G, B) color
+		// Convert from "rgb(R, G, B)" string to RGB array
+		$tmpColor = array_splice($matches, 1);
+	    } else if ($color[0] === '#') {
+		$color = substr($color, 1);
+		// HEX Color, convert to RGB array.
+		$tmpColor = sscanf($color, "%02X%02X%02X");
+	    } else {
+		PHPUnit_Framework_Assert::fail("The acceptance test does not know how to handle the color string : '$color'. "
+			. "Please provide # before HEX colors in your features.");
+	    }
+	    return $tmpColor;
+	}
+
 	/**
 	 * @Then I see that the header color is :color
 	 */
 	public function iSeeThatTheHeaderColorIs($color) {
 		$headerColor = $this->actor->getSession()->evaluateScript("return $('#header').css('background-color');");
-
-		if ($headerColor[0] === '#') {
-			$headerColor = substr($headerColor, 1);
-		} else if (preg_match("/rgb\(\s*(\d+),\s*(\d+),\s*(\d+)\)/", $headerColor, $matches)) {
-			// Convert from hex string to RGB array
-			$color = sscanf($color, "%02X%02X%02X");
-
-			// Convert from "rgb(R, G, B)" string to RGB array
-			$headerColor = array_splice($matches, 1);
-		} else {
-			PHPUnit_Framework_Assert::fail("The acceptance test does not know how to handle the color string returned by the browser: $headerColor");
-		}
-
+		$headerColor = $this->getRGBArray($headerColor);
+		$color = $this->getRGBArray($color);
 		PHPUnit_Framework_Assert::assertEquals($color, $headerColor);
 	}