From b51f2a60800829ee7585f10451a41bc4db0359a9 Mon Sep 17 00:00:00 2001 From: Winnie Hellmann Date: Fri, 5 May 2017 22:47:32 +0000 Subject: Colorize labels in issue search field --- .../filtered_search_visual_tokens.js | 40 ++++++- app/assets/javascripts/lib/utils/ajax_cache.js | 32 +++++ app/assets/stylesheets/framework/filters.scss | 14 ++- app/assets/stylesheets/framework/variables.scss | 2 + app/controllers/dashboard/labels_controller.rb | 2 +- app/controllers/groups/labels_controller.rb | 2 +- app/controllers/projects/labels_controller.rb | 2 +- app/serializers/label_entity.rb | 1 + app/serializers/label_serializer.rb | 7 ++ changelogs/unreleased/winh-visual-token-labels.yml | 4 + .../filtered_search_visual_tokens_spec.js | 101 ++++++++++++++++ spec/javascripts/fixtures/labels.rb | 56 +++++++++ spec/javascripts/lib/utils/ajax_cache_spec.js | 129 +++++++++++++++++++++ spec/serializers/label_serializer_spec.rb | 46 ++++++++ 14 files changed, 431 insertions(+), 7 deletions(-) create mode 100644 app/assets/javascripts/lib/utils/ajax_cache.js create mode 100644 app/serializers/label_serializer.rb create mode 100644 changelogs/unreleased/winh-visual-token-labels.yml create mode 100644 spec/javascripts/fixtures/labels.rb create mode 100644 spec/javascripts/lib/utils/ajax_cache_spec.js create mode 100644 spec/serializers/label_serializer_spec.rb diff --git a/app/assets/javascripts/filtered_search/filtered_search_visual_tokens.js b/app/assets/javascripts/filtered_search/filtered_search_visual_tokens.js index 59ce0587e1e..f3003b86493 100644 --- a/app/assets/javascripts/filtered_search/filtered_search_visual_tokens.js +++ b/app/assets/javascripts/filtered_search/filtered_search_visual_tokens.js @@ -1,3 +1,5 @@ +import AjaxCache from '~/lib/utils/ajax_cache'; +import '~/flash'; /* global Flash */ import FilteredSearchContainer from './container'; class FilteredSearchVisualTokens { @@ -48,6 +50,40 @@ class FilteredSearchVisualTokens { `; } + static updateLabelTokenColor(tokenValueContainer, tokenValue) { + const filteredSearchInput = FilteredSearchContainer.container.querySelector('.filtered-search'); + const baseEndpoint = filteredSearchInput.dataset.baseEndpoint; + const labelsEndpoint = `${baseEndpoint}/labels.json`; + + return AjaxCache.retrieve(labelsEndpoint) + .then((labels) => { + const matchingLabel = (labels || []).find(label => `~${gl.DropdownUtils.getEscapedText(label.title)}` === tokenValue); + + if (!matchingLabel) { + return; + } + + const tokenValueStyle = tokenValueContainer.style; + tokenValueStyle.backgroundColor = matchingLabel.color; + tokenValueStyle.color = matchingLabel.text_color; + + if (matchingLabel.text_color === '#FFFFFF') { + const removeToken = tokenValueContainer.querySelector('.remove-token'); + removeToken.classList.add('inverted'); + } + }) + .catch(() => new Flash('An error occurred while fetching label colors.')); + } + + static renderVisualTokenValue(parentElement, tokenName, tokenValue) { + const tokenValueContainer = parentElement.querySelector('.value-container'); + tokenValueContainer.querySelector('.value').innerText = tokenValue; + + if (tokenName.toLowerCase() === 'label') { + FilteredSearchVisualTokens.updateLabelTokenColor(tokenValueContainer, tokenValue); + } + } + static addVisualTokenElement(name, value, isSearchTerm) { const li = document.createElement('li'); li.classList.add('js-visual-token'); @@ -55,7 +91,7 @@ class FilteredSearchVisualTokens { if (value) { li.innerHTML = FilteredSearchVisualTokens.createVisualTokenElementHTML(); - li.querySelector('.value').innerText = value; + FilteredSearchVisualTokens.renderVisualTokenValue(li, name, value); } else { li.innerHTML = '
'; } @@ -74,7 +110,7 @@ class FilteredSearchVisualTokens { const name = FilteredSearchVisualTokens.getLastTokenPartial(); lastVisualToken.innerHTML = FilteredSearchVisualTokens.createVisualTokenElementHTML(); lastVisualToken.querySelector('.name').innerText = name; - lastVisualToken.querySelector('.value').innerText = value; + FilteredSearchVisualTokens.renderVisualTokenValue(lastVisualToken, name, value); } } diff --git a/app/assets/javascripts/lib/utils/ajax_cache.js b/app/assets/javascripts/lib/utils/ajax_cache.js new file mode 100644 index 00000000000..d99eefb5089 --- /dev/null +++ b/app/assets/javascripts/lib/utils/ajax_cache.js @@ -0,0 +1,32 @@ +const AjaxCache = { + internalStorage: { }, + get(endpoint) { + return this.internalStorage[endpoint]; + }, + hasData(endpoint) { + return Object.prototype.hasOwnProperty.call(this.internalStorage, endpoint); + }, + purge(endpoint) { + delete this.internalStorage[endpoint]; + }, + retrieve(endpoint) { + if (AjaxCache.hasData(endpoint)) { + return Promise.resolve(AjaxCache.get(endpoint)); + } + + return new Promise((resolve, reject) => { + $.ajax(endpoint) // eslint-disable-line promise/catch-or-return + .then(data => resolve(data), + (jqXHR, textStatus, errorThrown) => { + const error = new Error(`${endpoint}: ${errorThrown}`); + error.textStatus = textStatus; + reject(error); + }, + ); + }) + .then((data) => { this.internalStorage[endpoint] = data; }) + .then(() => AjaxCache.get(endpoint)); + }, +}; + +export default AjaxCache; diff --git a/app/assets/stylesheets/framework/filters.scss b/app/assets/stylesheets/framework/filters.scss index 0692f65043b..e624d0d951e 100644 --- a/app/assets/stylesheets/framework/filters.scss +++ b/app/assets/stylesheets/framework/filters.scss @@ -114,11 +114,21 @@ padding-right: 8px; .fa-close { - color: $gl-text-color-disabled; + color: $gl-text-color-secondary; } &:hover .fa-close { - color: $gl-text-color-secondary; + color: $gl-text-color; + } + + &.inverted { + .fa-close { + color: $gl-text-color-secondary-inverted; + } + + &:hover .fa-close { + color: $gl-text-color-inverted; + } } } diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index 49741c963df..08bcb582613 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -101,6 +101,8 @@ $gl-font-size: 14px; $gl-text-color: rgba(0, 0, 0, .85); $gl-text-color-secondary: rgba(0, 0, 0, .55); $gl-text-color-disabled: rgba(0, 0, 0, .35); +$gl-text-color-inverted: rgba(255, 255, 255, 1.0); +$gl-text-color-secondary-inverted: rgba(255, 255, 255, .85); $gl-text-green: $green-600; $gl-text-red: $red-500; $gl-text-orange: $orange-600; diff --git a/app/controllers/dashboard/labels_controller.rb b/app/controllers/dashboard/labels_controller.rb index d5031da867a..dd1d46a68c7 100644 --- a/app/controllers/dashboard/labels_controller.rb +++ b/app/controllers/dashboard/labels_controller.rb @@ -3,7 +3,7 @@ class Dashboard::LabelsController < Dashboard::ApplicationController labels = LabelsFinder.new(current_user).execute respond_to do |format| - format.json { render json: labels.as_json(only: [:id, :title, :color]) } + format.json { render json: LabelSerializer.new.represent_appearance(labels) } end end end diff --git a/app/controllers/groups/labels_controller.rb b/app/controllers/groups/labels_controller.rb index facb25525b5..3fa0516fb0c 100644 --- a/app/controllers/groups/labels_controller.rb +++ b/app/controllers/groups/labels_controller.rb @@ -15,7 +15,7 @@ class Groups::LabelsController < Groups::ApplicationController format.json do available_labels = LabelsFinder.new(current_user, group_id: @group.id).execute - render json: available_labels.as_json(only: [:id, :title, :color]) + render json: LabelSerializer.new.represent_appearance(available_labels) end end end diff --git a/app/controllers/projects/labels_controller.rb b/app/controllers/projects/labels_controller.rb index 2f55ba4e700..71bfb7163da 100644 --- a/app/controllers/projects/labels_controller.rb +++ b/app/controllers/projects/labels_controller.rb @@ -19,7 +19,7 @@ class Projects::LabelsController < Projects::ApplicationController respond_to do |format| format.html format.json do - render json: @available_labels.as_json(only: [:id, :title, :color]) + render json: LabelSerializer.new.represent_appearance(@available_labels) end end end diff --git a/app/serializers/label_entity.rb b/app/serializers/label_entity.rb index 304fd9de08f..ad565654342 100644 --- a/app/serializers/label_entity.rb +++ b/app/serializers/label_entity.rb @@ -6,6 +6,7 @@ class LabelEntity < Grape::Entity expose :group_id expose :project_id expose :template + expose :text_color expose :created_at expose :updated_at end diff --git a/app/serializers/label_serializer.rb b/app/serializers/label_serializer.rb new file mode 100644 index 00000000000..ad6ba8c46c9 --- /dev/null +++ b/app/serializers/label_serializer.rb @@ -0,0 +1,7 @@ +class LabelSerializer < BaseSerializer + entity LabelEntity + + def represent_appearance(resource) + represent(resource, { only: [:id, :title, :color, :text_color] }) + end +end diff --git a/changelogs/unreleased/winh-visual-token-labels.yml b/changelogs/unreleased/winh-visual-token-labels.yml new file mode 100644 index 00000000000..d4952e910b4 --- /dev/null +++ b/changelogs/unreleased/winh-visual-token-labels.yml @@ -0,0 +1,4 @@ +--- +title: Colorize labels in search field +merge_request: 11047 +author: diff --git a/spec/javascripts/filtered_search/filtered_search_visual_tokens_spec.js b/spec/javascripts/filtered_search/filtered_search_visual_tokens_spec.js index d75b9061281..8b750561eb7 100644 --- a/spec/javascripts/filtered_search/filtered_search_visual_tokens_spec.js +++ b/spec/javascripts/filtered_search/filtered_search_visual_tokens_spec.js @@ -1,3 +1,5 @@ +import AjaxCache from '~/lib/utils/ajax_cache'; + require('~/filtered_search/filtered_search_visual_tokens'); const FilteredSearchSpecHelper = require('../helpers/filtered_search_spec_helper'); @@ -611,4 +613,103 @@ describe('Filtered Search Visual Tokens', () => { expect(token.querySelector('.value').innerText).toEqual('~bug'); }); }); + + describe('renderVisualTokenValue', () => { + let searchTokens; + + beforeEach(() => { + tokensContainer.innerHTML = FilteredSearchSpecHelper.createTokensContainerHTML(` + ${FilteredSearchSpecHelper.createFilterVisualTokenHTML('label', 'none')} + ${FilteredSearchSpecHelper.createSearchVisualTokenHTML('search')} + ${FilteredSearchSpecHelper.createFilterVisualTokenHTML('milestone', 'upcoming')} + `); + + searchTokens = document.querySelectorAll('.filtered-search-token'); + }); + + it('renders a token value element', () => { + spyOn(gl.FilteredSearchVisualTokens, 'updateLabelTokenColor'); + const updateLabelTokenColorSpy = gl.FilteredSearchVisualTokens.updateLabelTokenColor; + + expect(searchTokens.length).toBe(2); + Array.prototype.forEach.call(searchTokens, (token) => { + updateLabelTokenColorSpy.calls.reset(); + + const tokenName = token.querySelector('.name').innerText; + const tokenValue = 'new value'; + gl.FilteredSearchVisualTokens.renderVisualTokenValue(token, tokenName, tokenValue); + + const tokenValueElement = token.querySelector('.value'); + expect(tokenValueElement.innerText).toBe(tokenValue); + + if (tokenName.toLowerCase() === 'label') { + const tokenValueContainer = token.querySelector('.value-container'); + expect(updateLabelTokenColorSpy.calls.count()).toBe(1); + const expectedArgs = [tokenValueContainer, tokenValue]; + expect(updateLabelTokenColorSpy.calls.argsFor(0)).toEqual(expectedArgs); + } else { + expect(updateLabelTokenColorSpy.calls.count()).toBe(0); + } + }); + }); + }); + + describe('updateLabelTokenColor', () => { + const jsonFixtureName = 'labels/project_labels.json'; + const dummyEndpoint = '/dummy/endpoint'; + + preloadFixtures(jsonFixtureName); + const labelData = getJSONFixture(jsonFixtureName); + const findLabel = tokenValue => labelData.find( + label => tokenValue === `~${gl.DropdownUtils.getEscapedText(label.title)}`, + ); + + const bugLabelToken = FilteredSearchSpecHelper.createFilterVisualToken('label', '~bug'); + const missingLabelToken = FilteredSearchSpecHelper.createFilterVisualToken('label', '~doesnotexist'); + const spaceLabelToken = FilteredSearchSpecHelper.createFilterVisualToken('label', '~"some space"'); + + const parseColor = (color) => { + const dummyElement = document.createElement('div'); + dummyElement.style.color = color; + return dummyElement.style.color; + }; + + beforeEach(() => { + tokensContainer.innerHTML = FilteredSearchSpecHelper.createTokensContainerHTML(` + ${bugLabelToken.outerHTML} + ${missingLabelToken.outerHTML} + ${spaceLabelToken.outerHTML} + `); + + const filteredSearchInput = document.querySelector('.filtered-search'); + filteredSearchInput.dataset.baseEndpoint = dummyEndpoint; + + AjaxCache.internalStorage = { }; + AjaxCache.internalStorage[`${dummyEndpoint}/labels.json`] = labelData; + }); + + const testCase = (token, done) => { + const tokenValueContainer = token.querySelector('.value-container'); + const tokenValue = token.querySelector('.value').innerText; + const label = findLabel(tokenValue); + + gl.FilteredSearchVisualTokens.updateLabelTokenColor(tokenValueContainer, tokenValue) + .then(() => { + if (label) { + expect(tokenValueContainer.getAttribute('style')).not.toBe(null); + expect(tokenValueContainer.style.backgroundColor).toBe(parseColor(label.color)); + expect(tokenValueContainer.style.color).toBe(parseColor(label.text_color)); + } else { + expect(token).toBe(missingLabelToken); + expect(tokenValueContainer.getAttribute('style')).toBe(null); + } + }) + .then(done) + .catch(fail); + }; + + it('updates the color of a label token', done => testCase(bugLabelToken, done)); + it('updates the color of a label token with spaces', done => testCase(spaceLabelToken, done)); + it('does not change color of a missing label', done => testCase(missingLabelToken, done)); + }); }); diff --git a/spec/javascripts/fixtures/labels.rb b/spec/javascripts/fixtures/labels.rb new file mode 100644 index 00000000000..2e4811b64a4 --- /dev/null +++ b/spec/javascripts/fixtures/labels.rb @@ -0,0 +1,56 @@ +require 'spec_helper' + +describe 'Labels (JavaScript fixtures)' do + include JavaScriptFixturesHelpers + + let(:admin) { create(:admin) } + let(:group) { create(:group, name: 'frontend-fixtures-group' )} + let(:project) { create(:project_empty_repo, namespace: group, path: 'labels-project') } + + let!(:project_label_bug) { create(:label, project: project, title: 'bug', color: '#FF0000') } + let!(:project_label_enhancement) { create(:label, project: project, title: 'enhancement', color: '#00FF00') } + let!(:project_label_feature) { create(:label, project: project, title: 'feature', color: '#0000FF') } + + let!(:group_label_roses) { create(:group_label, group: group, title: 'roses', color: '#FF0000') } + let!(:groub_label_space) { create(:group_label, group: group, title: 'some space', color: '#FFFFFF') } + let!(:groub_label_violets) { create(:group_label, group: group, title: 'violets', color: '#0000FF') } + + before(:all) do + clean_frontend_fixtures('labels/') + end + + describe Groups::LabelsController, '(JavaScript fixtures)', type: :controller do + render_views + + before(:each) do + sign_in(admin) + end + + it 'labels/group_labels.json' do |example| + get :index, + group_id: group, + format: 'json' + + expect(response).to be_success + store_frontend_fixture(response, example.description) + end + end + + describe Projects::LabelsController, '(JavaScript fixtures)', type: :controller do + render_views + + before(:each) do + sign_in(admin) + end + + it 'labels/project_labels.json' do |example| + get :index, + namespace_id: group, + project_id: project, + format: 'json' + + expect(response).to be_success + store_frontend_fixture(response, example.description) + end + end +end diff --git a/spec/javascripts/lib/utils/ajax_cache_spec.js b/spec/javascripts/lib/utils/ajax_cache_spec.js new file mode 100644 index 00000000000..7b466a11b92 --- /dev/null +++ b/spec/javascripts/lib/utils/ajax_cache_spec.js @@ -0,0 +1,129 @@ +import AjaxCache from '~/lib/utils/ajax_cache'; + +describe('AjaxCache', () => { + const dummyEndpoint = '/AjaxCache/dummyEndpoint'; + const dummyResponse = { + important: 'dummy data', + }; + let ajaxSpy = (url) => { + expect(url).toBe(dummyEndpoint); + const deferred = $.Deferred(); + deferred.resolve(dummyResponse); + return deferred.promise(); + }; + + beforeEach(() => { + AjaxCache.internalStorage = { }; + spyOn(jQuery, 'ajax').and.callFake(url => ajaxSpy(url)); + }); + + describe('#get', () => { + it('returns undefined if cache is empty', () => { + const data = AjaxCache.get(dummyEndpoint); + + expect(data).toBe(undefined); + }); + + it('returns undefined if cache contains no matching data', () => { + AjaxCache.internalStorage['not matching'] = dummyResponse; + + const data = AjaxCache.get(dummyEndpoint); + + expect(data).toBe(undefined); + }); + + it('returns matching data', () => { + AjaxCache.internalStorage[dummyEndpoint] = dummyResponse; + + const data = AjaxCache.get(dummyEndpoint); + + expect(data).toBe(dummyResponse); + }); + }); + + describe('#hasData', () => { + it('returns false if cache is empty', () => { + expect(AjaxCache.hasData(dummyEndpoint)).toBe(false); + }); + + it('returns false if cache contains no matching data', () => { + AjaxCache.internalStorage['not matching'] = dummyResponse; + + expect(AjaxCache.hasData(dummyEndpoint)).toBe(false); + }); + + it('returns true if data is available', () => { + AjaxCache.internalStorage[dummyEndpoint] = dummyResponse; + + expect(AjaxCache.hasData(dummyEndpoint)).toBe(true); + }); + }); + + describe('#purge', () => { + it('does nothing if cache is empty', () => { + AjaxCache.purge(dummyEndpoint); + + expect(AjaxCache.internalStorage).toEqual({ }); + }); + + it('does nothing if cache contains no matching data', () => { + AjaxCache.internalStorage['not matching'] = dummyResponse; + + AjaxCache.purge(dummyEndpoint); + + expect(AjaxCache.internalStorage['not matching']).toBe(dummyResponse); + }); + + it('removes matching data', () => { + AjaxCache.internalStorage[dummyEndpoint] = dummyResponse; + + AjaxCache.purge(dummyEndpoint); + + expect(AjaxCache.internalStorage).toEqual({ }); + }); + }); + + describe('#retrieve', () => { + it('stores and returns data from Ajax call if cache is empty', (done) => { + AjaxCache.retrieve(dummyEndpoint) + .then((data) => { + expect(data).toBe(dummyResponse); + expect(AjaxCache.internalStorage[dummyEndpoint]).toBe(dummyResponse); + }) + .then(done) + .catch(fail); + }); + + it('returns undefined if Ajax call fails and cache is empty', (done) => { + const dummyStatusText = 'exploded'; + const dummyErrorMessage = 'server exploded'; + ajaxSpy = (url) => { + expect(url).toBe(dummyEndpoint); + const deferred = $.Deferred(); + deferred.reject(null, dummyStatusText, dummyErrorMessage); + return deferred.promise(); + }; + + AjaxCache.retrieve(dummyEndpoint) + .then(data => fail(`Received unexpected data: ${JSON.stringify(data)}`)) + .catch((error) => { + expect(error.message).toBe(`${dummyEndpoint}: ${dummyErrorMessage}`); + expect(error.textStatus).toBe(dummyStatusText); + done(); + }) + .catch(fail); + }); + + it('makes no Ajax call if matching data exists', (done) => { + AjaxCache.internalStorage[dummyEndpoint] = dummyResponse; + ajaxSpy = () => fail(new Error('expected no Ajax call!')); + + AjaxCache.retrieve(dummyEndpoint) + .then((data) => { + expect(data).toBe(dummyResponse); + }) + .then(done) + .catch(fail); + }); + }); +}); diff --git a/spec/serializers/label_serializer_spec.rb b/spec/serializers/label_serializer_spec.rb new file mode 100644 index 00000000000..c58c7da1f9e --- /dev/null +++ b/spec/serializers/label_serializer_spec.rb @@ -0,0 +1,46 @@ +require 'spec_helper' + +describe LabelSerializer do + let(:user) { create(:user) } + + let(:serializer) do + described_class.new(user: user) + end + + subject { serializer.represent(resource) } + + describe '#represent' do + context 'when a single object is being serialized' do + let(:resource) { create(:label) } + + it 'serializes the label object' do + expect(subject[:id]).to eq resource.id + end + end + + context 'when multiple objects are being serialized' do + let(:num_labels) { 2 } + let(:resource) { create_list(:label, num_labels) } + + it 'serializes the array of labels' do + expect(subject.size).to eq(num_labels) + end + end + end + + describe '#represent_appearance' do + context 'when represents only appearance' do + let(:resource) { create(:label) } + + subject { serializer.represent_appearance(resource) } + + it 'serializes only attributes used for appearance' do + expect(subject.keys).to eq([:id, :title, :color, :text_color]) + expect(subject[:id]).to eq(resource.id) + expect(subject[:title]).to eq(resource.title) + expect(subject[:color]).to eq(resource.color) + expect(subject[:text_color]).to eq(resource.text_color) + end + end + end +end -- cgit v1.2.1