Fixes #2641 - Can't use HEX/HSL color value in KB Admin color picker.

This commit is contained in:
Mantas Masalskis 2020-02-19 16:02:28 +01:00 committed by Thorsten Eckel
parent bcce8c51f9
commit 6c60021196
18 changed files with 3185 additions and 90 deletions

View file

@ -1,10 +1,10 @@
# coffeelint: disable=camel_case_classes # coffeelint: disable=camel_case_classes
class App.Color extends Spine.Controller class App.Color extends Spine.Controller
hsl: undefined color: undefined
moving: undefined
elements: elements:
'.js-input': 'input' '.js-input': 'input'
'.js-shadow': 'shadow'
'.js-swatch': 'swatch' '.js-swatch': 'swatch'
'.js-colorpicker-hue-saturation': 'hueSaturation' '.js-colorpicker-hue-saturation': 'hueSaturation'
'.js-colorpicker-lightness-plane': 'lightnessPlane' '.js-colorpicker-lightness-plane': 'lightnessPlane'
@ -31,110 +31,65 @@ class App.Color extends Spine.Controller
@el @el
render: -> render: ->
@hsl = @rgbToHsl(@parseColor(@attribute.value)) @color = new App.ColorObject(@attribute.value)
@html App.view('generic/color') @html App.view('generic/color')
attribute: @attribute attribute: @attribute
hsl: @hsl hsl: @color.asHslArray()
onInput: -> onInput: ->
@update @input.val() @color.updateWithString @input.val()
@output()
update: (color) ->
@updateSwatch(color)
@hsl = @rgbToHsl(@parseColor(color))
@renderPicker() @renderPicker()
@updateSwatch @color.asString()
updateSwatch: (color) -> updateSwatch: (colorString) ->
@swatch.css 'background-color', '' @swatch.css 'background-color', ''
@swatch.css 'background-color', color @swatch.css 'background-color', colorString
output: ->
hslString = @hslString(@hsl)
@input.val hslString
@updateSwatch hslString
@shadow.val @rgbToHex(@parseColor(hslString))
componentToHex: (c) ->
hex = c.toString(16)
if hex.length == 1 then '0' + hex else hex
rgbToHex: (rgba) ->
'#' + @componentToHex(rgba[0]) + @componentToHex(rgba[1]) + @componentToHex(rgba[2])
parseColor: (color) ->
canvas = document.createElement('canvas')
canvas.width = canvas.height = 1
ctx = canvas.getContext('2d')
ctx.clearRect(0, 0, 1, 1)
ctx.fillStyle = color
ctx.fillRect(0, 0, 1, 1)
ctx.getImageData(0, 0, 1, 1).data
rgbToHsl: (rgb) ->
return [0, 0, 0] if !rgb
r = rgb[0] / 255
g = rgb[1] / 255
b = rgb[2] / 255
max = Math.max(r, g, b)
min = Math.min(r, g, b)
l = (max + min) / 2
if (max == min)
h = s = 0 # achromatic
else
d = max - min
s = if l > 0.5 then d / (2 - max - min) else d / (max + min)
h = switch
when r is max then (g - b) / d + (g < b ? 6 : 0)
when g is max then (b - r) / d + 2
when b is max then (r - g) / d + 4
h /= 6
[h, s, l]
hslString: ->
"hsl(#{Math.round(360 * @hsl[0])},#{Math.round(100 * @hsl[1])}%,#{Math.round(100 * @hsl[2])}%)"
onHueSaturationMousedown: (event) -> onHueSaturationMousedown: (event) ->
@offset = @hueSaturation.offset()
$(document).on 'mousemove.colorpicker', @onHueSaturationMousemove $(document).on 'mousemove.colorpicker', @onHueSaturationMousemove
$(document).on 'mouseup.colorpicker', @onMouseup $(document).on 'mouseup.colorpicker', @onMouseup
@onHueSaturationMousemove(event) @onHueSaturationMousemove(event)
onHueSaturationMousemove: (event) => onHueSaturationMousemove: (event) =>
@hsl[0] = Math.max(0, Math.min(1, (event.pageX - @offset.left)/@hueSaturation.width())) offset = @hueSaturation.offset()
@hsl[1] = Math.max(0, Math.min(1, 1-(event.pageY - @offset.top)/@hueSaturation.height()))
@color.updateWithHslComponent Math.max(0, Math.min(1, (event.pageX - offset.left)/@hueSaturation.width())), 0
@color.updateWithHslComponent Math.max(0, Math.min(1, 1-(event.pageY - offset.top)/@hueSaturation.height())), 1
@renderPicker() @renderPicker()
@output() @renderPickerOutput()
onLightnessMousedown: (event) -> onLightnessMousedown: (event) ->
@offset = @lightness.offset()
$(document).on 'mousemove.colorpicker', @onLightnessMousemove $(document).on 'mousemove.colorpicker', @onLightnessMousemove
$(document).on 'mouseup.colorpicker', @onMouseup $(document).on 'mouseup.colorpicker', @onMouseup
@onLightnessMousemove(event) @onLightnessMousemove(event)
onLightnessMousemove: (event) => onLightnessMousemove: (event) =>
@hsl[2] = Math.max(0, Math.min(1, 1-(event.pageY - @offset.top)/@lightness.height())) offset = @lightness.offset()
@color.updateWithHslComponent Math.max(0, Math.min(1, 1-(event.pageY - offset.top)/@lightness.height())), 2
@renderPicker() @renderPicker()
@output() @renderPickerOutput()
onMouseup: -> onMouseup: ->
$(document).off 'mousemove.colorpicker' $(document).off 'mousemove.colorpicker'
$(document).off 'mouseup.colorpicker' $(document).off 'mouseup.colorpicker'
renderPickerOutput: ->
colorString = @color.asString()
@updateSwatch colorString
@input.val colorString
renderPicker: -> renderPicker: ->
@lightnessPlane.css 'background-color': "hsla(0,0%,#{if @hsl[2] > 0.5 then 100 else 0}%,#{2*Math.abs(@hsl[2]-0.5)})" components = @color.asHslArray()
@saturationGradient.css 'background-image': "linear-gradient(transparent, hsl(0, 0%, #{@hsl[2]*100}%))"
@lightnessPlane.css 'background-color': "hsla(0,0%,#{if components[2] > 0.5 then 100 else 0}%,#{2*Math.abs(components[2]-0.5)})"
@saturationGradient.css 'background-image': "linear-gradient(transparent, hsl(0, 0%, #{components[2]*100}%))"
@circle.css @circle.css
left: @hsl[0]*100 +'%' left: components[0]*100 +'%'
top: 100 - @hsl[1]*100 +'%' top: 100 - components[1]*100 +'%'
borderColor: if @hsl[2] > 0.5 then 'black' else 'white' borderColor: if components[2] > 0.5 then 'black' else 'white'
@huePlane.css 'background-color': "hsl(#{@hsl[0]*360}, 100%, 50%)" @huePlane.css 'background-color': "hsl(#{components[0]*360}, 100%, 50%)"
@slider.css top: 100 - @hsl[2]*100 +'%' @slider.css top: 100 - components[2]*100 +'%'

View file

@ -0,0 +1,70 @@
class App.ColorObject
original: undefined
constructor: (original) ->
@original = original
asHslArray: ->
if _.isArray(@original)
@original
else
@constructor.anyToHslArray @original
asString: ->
if _.isArray(@original)
@constructor.hslArrayToHslString(@original)
else
@original
updateWithString: (newValue) ->
@original = newValue
updateWithHslComponent: (value, index) ->
if !_.isArray(@original)
@original = @constructor.anyToHslArray @original
@original[index] = value
@anyToRgb: (color) ->
canvas = document.createElement('canvas')
canvas.width = canvas.height = 1
ctx = canvas.getContext('2d')
ctx.clearRect(0, 0, 1, 1)
ctx.fillStyle = color
ctx.fillRect(0, 0, 1, 1)
ctx.getImageData(0, 0, 1, 1).data
@anyToHslArray: (color) ->
@rgbToHslArray @anyToRgb(color)
@anyToHslString: (color) ->
@hslArrayToHslString @rgbToHslArray @anyToRgb color
@rgbToHslArray: (rgb) ->
return [0, 0, 0] if !rgb
r = rgb[0] / 255
g = rgb[1] / 255
b = rgb[2] / 255
max = Math.max(r, g, b)
min = Math.min(r, g, b)
l = (max + min) / 2
if (max == min)
h = s = 0 # achromatic
else
d = max - min
s = if l > 0.5 then d / (2 - max - min) else d / (max + min)
h = switch
when r is max then (g - b) / d + (g < b ? 6 : 0)
when g is max then (b - r) / d + 2
when b is max then (r - g) / d + 4
h /= 6
[h, s, l]
@hslArrayToHslString: (hslArray) ->
"hsl(#{Math.round(360 * hslArray[0])},#{Math.round(100 * hslArray[1])}%,#{Math.round(100 * hslArray[2])}%)"

View file

@ -1,6 +1,5 @@
<div class="color controls controls--button dropdown-toggle" data-toggle="dropdown"> <div class="color controls controls--button dropdown-toggle" data-toggle="dropdown">
<input class="color-shadow js-shadow" name="<%= @attribute.name %>" value="<%= @attribute.value %>"> <input id="<%= @attribute.id %>" name="<%= @attribute.name %>" type="text" value="<%= @attribute.value %>" class="<%= @attribute.class %> js-input" <% if @attribute.placeholder: %>placeholder="<%- @Ti(@attribute.placeholder) %>"<% end %> <%= @attribute.required %> <%= @attribute.autofocus %> <%- @attribute.autocapitalize %> <%- @attribute.autocomplete %> <% if @attribute.min isnt undefined: %> min="<%= @attribute.min %>"<% end %><% if @attribute.max isnt undefined: %> max="<%= @attribute.max %>"<% end %><% if @attribute.step: %> step="<%= @attribute.step %>"<% end %><% if @attribute.disabled: %> disabled<% end %>>
<input id="<%= @attribute.id %>" type="text" value="<%= @attribute.value %>" class="<%= @attribute.class %> js-input" <% if @attribute.placeholder: %>placeholder="<%- @Ti(@attribute.placeholder) %>"<% end %> <%= @attribute.required %> <%= @attribute.autofocus %> <%- @attribute.autocapitalize %> <%- @attribute.autocomplete %> <% if @attribute.min isnt undefined: %> min="<%= @attribute.min %>"<% end %><% if @attribute.max isnt undefined: %> max="<%= @attribute.max %>"<% end %><% if @attribute.step: %> step="<%= @attribute.step %>"<% end %><% if @attribute.disabled: %> disabled<% end %>>
<div class="controls-button"> <div class="controls-button">
<div class="controls-button-inner"> <div class="controls-button-inner">
<div class="color-field js-swatch" style="background-color: <%= @attribute.value %>"></div> <div class="color-field js-swatch" style="background-color: <%= @attribute.value %>"></div>

View file

@ -10684,10 +10684,6 @@ output {
padding: 4px; padding: 4px;
} }
.color-shadow {
display: none;
}
.color-field { .color-field {
width: 31px; width: 31px;
height: 100%; height: 100%;

View file

@ -26,8 +26,9 @@ class KnowledgeBase < ApplicationModel
validates :category_layout, inclusion: { in: KnowledgeBase::LAYOUTS } validates :category_layout, inclusion: { in: KnowledgeBase::LAYOUTS }
validates :homepage_layout, inclusion: { in: KnowledgeBase::LAYOUTS } validates :homepage_layout, inclusion: { in: KnowledgeBase::LAYOUTS }
validates :color_highlight, presence: true validates :color_highlight, presence: true, color: true
validates :color_header, presence: true validates :color_header, presence: true, color: true
validates :iconset, inclusion: { in: KnowledgeBase::ICONSETS } validates :iconset, inclusion: { in: KnowledgeBase::ICONSETS }
scope :active, -> { where(active: true) } scope :active, -> { where(active: true) }

View file

@ -0,0 +1,15 @@
<link rel="stylesheet" href="/assets/tests/qunit-1.21.0.css">
<script src="/assets/tests/qunit-1.21.0.js"></script>
<script src="/assets/tests/color_object.js"></script>
<style type="text/css">
body {
padding-top: 0px;
}
</style>
<script type="text/javascript">
</script>
<div id="qunit" class="u-dontfold"></div>

View file

@ -0,0 +1,24 @@
<link rel="stylesheet" href="/assets/tests/qunit-1.21.0.css">
<script src="/assets/tests/qunit-1.21.0.js"></script>
<script src="/assets/tests/syn-0.14.1.js"></script>
<script src="/assets/tests/form_color.js"></script>
<style type="text/css">
body {
padding-top: 0px;
}
</style>
<script type="text/javascript">
</script>
<div id="qunit" class="u-dontfold"></div>
<div>
<form class="form-stacked pull-left">
<div id="forms"></div>
<button type="submit" class="btn btn-primary submit">Submit</button>
</form>
</div>

View file

@ -15,6 +15,7 @@ Zammad::Application.routes.draw do
match '/tests_form_trim', to: 'tests#form_trim', via: :get match '/tests_form_trim', to: 'tests#form_trim', via: :get
match '/tests_form_extended', to: 'tests#form_extended', via: :get match '/tests_form_extended', to: 'tests#form_extended', via: :get
match '/tests_form_timer', to: 'tests#form_timer', via: :get match '/tests_form_timer', to: 'tests#form_timer', via: :get
match '/tests_form_color', to: 'tests#form_color', via: :get
match '/tests_form_validation', to: 'tests#form_validation', via: :get match '/tests_form_validation', to: 'tests#form_validation', via: :get
match '/tests_form_column_select', to: 'tests#form_column_select', via: :get match '/tests_form_column_select', to: 'tests#form_column_select', via: :get
match '/tests_form_searchable_select', to: 'tests#form_searchable_select', via: :get match '/tests_form_searchable_select', to: 'tests#form_searchable_select', via: :get
@ -25,6 +26,7 @@ Zammad::Application.routes.draw do
match '/tests_ticket_selector', to: 'tests#ticket_selector', via: :get match '/tests_ticket_selector', to: 'tests#ticket_selector', via: :get
match '/tests_taskbar', to: 'tests#taskbar', via: :get match '/tests_taskbar', to: 'tests#taskbar', via: :get
match '/tests_text_module', to: 'tests#text_module', via: :get match '/tests_text_module', to: 'tests#text_module', via: :get
match '/tests_color_object', to: 'tests#color_object', via: :get
match '/tests/wait/:sec', to: 'tests#wait', via: :get match '/tests/wait/:sec', to: 'tests#wait', via: :get
match '/tests/raised_exception', to: 'tests#error_raised_exception', via: :get match '/tests/raised_exception', to: 'tests#error_raised_exception', via: :get

View file

@ -6,8 +6,8 @@ class InitializeKnowledgeBase < ActiveRecord::Migration[5.0]
create_table :knowledge_bases do |t| create_table :knowledge_bases do |t|
t.string :iconset, limit: 30, null: false t.string :iconset, limit: 30, null: false
t.string :color_highlight, limit: 9, null: false t.string :color_highlight, limit: 25, null: false
t.string :color_header, limit: 9, null: false t.string :color_header, limit: 25, null: false
t.string :homepage_layout, null: false t.string :homepage_layout, null: false
t.string :category_layout, null: false t.string :category_layout, null: false

View file

@ -0,0 +1,8 @@
class Issue2641KbColorChangeLimit < ActiveRecord::Migration[5.2]
def change
return if !Setting.find_by(name: 'system_init_done')
change_column :knowledge_bases, :color_highlight, :string, limit: 25
change_column :knowledge_bases, :color_header, :string, limit: 25
end
end

19
lib/color_validator.rb Normal file
View file

@ -0,0 +1,19 @@
class ColorValidator < ActiveModel::EachValidator
REGEXP = {
RGB: /^rgb\((((((((1?[1-9]?\d)|10\d|(2[0-4]\d)|25[0-5]),\s?)){2}|((((1?[1-9]?\d)|10\d|(2[0-4]\d)|25[0-5])\s)){2})((1?[1-9]?\d)|10\d|(2[0-4]\d)|25[0-5]))|((((([1-9]?\d(\.\d+)?)|100|(\.\d+))%,\s?){2}|((([1-9]?\d(\.\d+)?)|100|(\.\d+))%\s){2})(([1-9]?\d(\.\d+)?)|100|(\.\d+))%))\)$/i,
HSL: /^hsl\(((((([12]?[1-9]?\d)|[12]0\d|(3[0-6]\d))(\.\d+)?)|(\.\d+))(deg)?|(0|0?\.\d+)turn|(([0-6](\.\d+)?)|(\.\d+))rad)((,\s?(([1-9]?\d(\.\d+)?)|100|(\.\d+))%){2}|(\s(([1-9]?\d(\.\d+)?)|100|(\.\d+))%){2})\)$/i,
HEX: /^#([\da-f]{3}){1,2}$/i
}.freeze
def validate_each(record, attribute, value)
return if color?(value)
record.errors[attribute] << (options[:message] || 'is not a color. Only Hex, RGB and HSL colors are supported.')
end
def color?(value)
sanitized_value = value.to_s.strip.gsub(', ', ',')
REGEXP.values.any? { |regexp| regexp.match? sanitized_value }
end
end

View file

@ -0,0 +1,21 @@
test('test color object', function() {
let hex = new App.ColorObject('#09f609')
let hsl = new App.ColorObject([0.5, 0.2, 0.3])
deepEqual(hex.asHslArray(), [1/3, 0.9294117647058824, 0.5], 'HEX converted to HSL components')
deepEqual(hsl.asHslArray(), [0.5, 0.2, 0.3], 'HSL components returned as original input')
equal(hex.asString(), '#09f609', 'HEX represented as original input')
equal(hsl.asString(), 'hsl(180,20%,30%)', 'HSL components represented as HSL string')
hex.updateWithString('#fff')
equal(hex.asString(), '#fff', 'color updated')
hsl.updateWithHslComponent(0.25, 1)
deepEqual(hsl.asHslArray(), [0.5, 0.25, 0.3], 'given HSL component updated')
deepEqual(Array.from(App.ColorObject.anyToRgb('#ff0000')), [255, 0, 0, 255], 'any to RGB')
deepEqual(App.ColorObject.anyToHslArray('#ff0000'), [0,1,0.5], 'any to HSL components')
equal(App.ColorObject.anyToHslString('#ff0000'), 'hsl(0,100%,50%)', 'any to HSL string')
deepEqual(App.ColorObject.rgbToHslArray([255, 0, 0]), [0,1,0.5], 'RGB to HSL components')
equal(App.ColorObject.hslArrayToHslString([0.5, 0.25, 0.3]), 'hsl(180,25%,30%)', 'HSL components to HSL string')
})

View file

@ -0,0 +1,111 @@
test("form elements check", function(assert) {
var done = assert.async(1)
$('#forms').append('<hr><h1>form elements check</h1><form id="form1"></form>')
var el = $('#form1')
var defaults = {
}
new App.ControllerForm({
el: el,
model: {
configure_attributes: [
{ name: 'color', display: 'Color', tag: 'color', null: false, default: '#fff' }
]
},
autofocus: true
});
var params = App.ControllerForm.params(el)
var test_params = {
color: '#fff'
}
deepEqual(params, test_params, 'default param check')
var inputEl = el.find('.js-input')[0]
var getSwatchColor = () => { return el.find('.js-swatch').css('background-color') }
var previousSwatchColor = undefined
new Promise( (resolve,reject) => {
syn.click(inputEl).type('[ctrl]+[a]+[backspace]', resolve)
})
.then( function() {
var params = App.ControllerForm.params(el)
var test_params = {
color: ''
}
deepEqual(params, test_params, 'UI allows color field to be empty')
})
.then( function() {
previousSwatchColor = getSwatchColor()
return new Promise( (resolve,reject) => {
syn.click(inputEl).type('rgb(0,100,100)', resolve)
})
})
.then( function() {
var params = App.ControllerForm.params(el)
var test_params = {
color: 'rgb(0,100,100)'
}
deepEqual(params, test_params, 'UI allows to type in RGB colors')
notEqual(previousSwatchColor, getSwatchColor(), 'color in swatch was updated')
})
.then( function() {
var circle = el.find('.js-colorpicker-circle')[0]
previousSwatchColor = getSwatchColor()
return new Promise( (resolve,reject) => {
syn.click(inputEl).drag(circle, { to: '-10x-10'}, resolve)
})
})
.then( function() {
var params = App.ControllerForm.params(el)
var test_params = {
color: 'hsl(169,100%,20%)'
}
deepEqual(params, test_params, 'Color is transformed to HSL after moving the circle')
notEqual(previousSwatchColor, getSwatchColor(), 'color in swatch was updated')
})
.then( function() {
var slider = el.find('.js-colorpicker-slider')[0]
previousSwatchColor = getSwatchColor()
return new Promise( (resolve,reject) => {
syn.drag(slider, { to: '-0x-10'}, resolve)
})
})
.then( function() {
var params = App.ControllerForm.params(el)
var test_params = {
color: 'hsl(169,100%,27%)'
}
deepEqual(params, test_params, 'Color code is changed after draging slider')
notEqual(previousSwatchColor, getSwatchColor(), 'color in swatch was updated')
})
.then( function() {
let circle = el.find('.js-colorpicker-circle').position()
let slider = el.find('.js-colorpicker-slider').position()
previousSwatchColor = getSwatchColor()
return new Promise( (resolve,reject) => {
syn.click(inputEl).type('[ctrl]+[a]+[backspace]#ff0000', () => {
syn.click(inputEl, resolve)
})
}).then(function() {
let new_circle = el.find('.js-colorpicker-circle').position()
let new_slider = el.find('.js-colorpicker-slider').position()
notDeepEqual(circle, new_circle, 'Color picker is updated after typing in color')
notDeepEqual(slider, new_slider, 'Color picker is updated after typing in color')
notEqual(previousSwatchColor, getSwatchColor(), 'color in swatch was updated')
})
})
.finally(done)
});

File diff suppressed because it is too large Load diff

View file

@ -0,0 +1,42 @@
require 'rails_helper'
RSpec.describe Issue2641KbColorChangeLimit, type: :db_migration do
subject(:knowledge_base) { create(:knowledge_base) }
before do
Setting.create_if_not_exists(
title: 'Kb active',
name: 'kb_active',
area: 'Kb::Core',
description: 'Defines if KB navbar button is enabled. Updated in KnowledgeBase callback.',
state: false,
preferences: {
prio: 1,
trigger: ['menu:render'],
authentication: true,
permission: ['admin.knowledge_base'],
},
frontend: true
)
Setting.create_if_not_exists(
title: 'Kb active publicly',
name: 'kb_active_publicly',
area: 'Kb::Core',
description: 'Defines if KB navbar button is enabled for users without KB permission. Updated in CanBePublished callback.',
state: false,
preferences: {
prio: 1,
trigger: ['menu:render'],
authentication: true,
permission: [],
},
frontend: true
)
end
it "doesn't change value for existing KB" do
expect { migrate }
.to not_change { knowledge_base.color_header }.and not_change { knowledge_base.color_highlight }
end
end

View file

@ -1,7 +1,7 @@
FactoryBot.define do FactoryBot.define do
factory 'knowledge_base/locale', aliases: %i[knowledge_base_locale] do factory 'knowledge_base/locale', aliases: %i[knowledge_base_locale] do
knowledge_base { nil } knowledge_base { nil }
system_locale { Locale.first } system_locale { Locale.first || create(:locale) }
before :create do |kb_locale| before :create do |kb_locale|
if kb_locale.knowledge_base.blank? if kb_locale.knowledge_base.blank?

View file

@ -63,4 +63,14 @@ RSpec.describe KnowledgeBase, type: :model do
end end
end end
end end
context 'acceptable colors' do
let(:allowed_values) { ['#aaa', '#ff0000', 'rgb(0,100,100)', 'hsl(0,100%,50%)'] }
let(:not_allowed_values) { ['aaa', '#aa', '#ff000', 'rgb(0,100,100', 'def(0,100%,0.5)', 'test'] }
it { is_expected.to allow_values(*allowed_values).for(:color_header) }
it { is_expected.to allow_values(*allowed_values).for(:color_highlight) }
it { is_expected.not_to allow_values(*not_allowed_values).for(:color_header) }
it { is_expected.not_to allow_values(*not_allowed_values).for(:color_highlight) }
end
end end

View file

@ -82,6 +82,10 @@ RSpec.describe 'QUnit', type: :system, authenticated: false, set_up: true, webso
q_unit_tests('form_timer') q_unit_tests('form_timer')
end end
it 'Color' do
q_unit_tests('form_color')
end
it 'Extended' do it 'Extended' do
q_unit_tests('form_extended') q_unit_tests('form_extended')
end end