HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Interdependent color swatch widgets

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
swatchinterdependentwidgetscolor

Problem

To illustrate some algorithmic problems I found while writing this code review, I needed to include a live demonstration, which I implemented using a Stack Snippet with Angular.js.

The demonstration takes a color input and a scalar parameter, and produces two read-only color outputs. The "Original" output is known to behave erratically (which is the point of the demo).

However, the HTML feels a bit repetitive. How can I use Angular more effectively? Optional bonus question: would your advice change if I wanted to present one of the outputs as HSV values instead of RGB?

I also think that the CSS is sloppy and not robust to various sizing conditions. Recommendations in that area would also be appreciated.

(For this review, I suggest treating the details of the color manipulation routines as black boxes. For those issues, you may comment on the original answer instead.)



`var ColorTweakerCtrl = function ColorTweakerCtrl($scope) {
$scope.inColor = new RGBColor(234, 150, 0);
$scope.diff = 127;

$scope.tweak = function tweak() {
$scope.inColor.r = Math.round($scope.inColor.r);
$scope.inColor.g = Math.round($scope.inColor.g);
$scope.inColor.b = Math.round($scope.inColor.b);
$scope.diff = Math.round($scope.diff);
$scope.outColor = tweakColor($scope.inColor, $scope.diff);
$scope.newColor = newTweakColor($scope.inColor, $scope.diff);
};
$scope.tweak();
};

function RGBColor(r, g, b) {
this.r = r; this.g = g; this.b = b;
}

RGBColor.prototype.toString = function() {
return 'rgb(' + Math.round(this.r) + ',' + Math.round(this.g) + ',' + Math.round(this.b) + ')';
};

/ Based on formulas from http://www.rapidtables.com/convert/color/rgb-to-hsv.htm /
RGBColor.prototype.toHSV = function toHSV() {
var r = this.r / 255,
g = this.g / 255,
b = this.b / 255;
var cMax = Math.max(r, g, b),
cMin = Math.min(r, g, b);
var Δ = cMax - cMin;
var hue = 60 * ( (cMax == r) ? ((g - b) / Δ) % 6
: (cMax ==

Solution

Markup

Your markup is invalid. A label is only allowed to be associated with a single form element.

A color input element would be a more semantically appropriate choice over that div.

CSS

For the RGB label text, I recommend switching to a mono-spaced font. As it stands, your form elements will not line up because each letter varies in width.

I don't see much reason for your number fields to be any wider than what's necessary for the the widest number value visible (which might not be the highest number!). A width of 3em-4em would probably be a good starting point.

Context

StackExchange Code Review Q#71429, answer score: 3

Revisions (0)

No revisions yet.