Colorize labels in issue search field

Screenshots

Before

After

Related Issues

Fixes #30467 (closed)

/cc @lbennett

Thanks @winniehell

marked as a Work In Progress

resolved all discussions

added 2 commits

• c7ea4b8a - Introduce AjaxCache class
• f071ca00 - Colorize labels in search field

Compare with previous version

resolved all discussions

mentioned in commit 870f712e

added 2 commits

• f753b60d - Introduce AjaxCache class
• 870f712e - Colorize labels in search field (!11047 (merged))

Compare with previous version

mentioned in merge request !10713

mentioned in commit 63ca1b5a

added 1744 commits

• 870f712e...c1e2da92 - 1742 commits from branch master
• 7e3ba5ff - Introduce AjaxCache class
• 63ca1b5a - Colorize labels in search field (!11047 (merged))

Compare with previous version

mentioned in commit 87d2b804

added 8 commits

• 63ca1b5a...4faa65d8 - 6 commits from branch master
• 5b8dde0e - Introduce AjaxCache class
• 87d2b804 - Colorize labels in search field (!11047 (merged))

Compare with previous version

mentioned in commit 2949a809

added 21 commits

• 87d2b804...ef71bf62 - 18 commits from branch master
• 7ba0b911 - Use LabelSerializer in label controllers
• f5177703 - Introduce AjaxCache class
• 2949a809 - Colorize labels in search field (!11047 (merged))

Compare with previous version

mentioned in commit 8275b927

added 3 commits

• ae08230c - Use LabelSerializer in label controllers
• 8008b5cf - Introduce AjaxCache class
• 8275b927 - Colorize labels in search field (!11047 (merged))

Compare with previous version

mentioned in commit cef34d48

added 10 commits

• 8275b927...2d43f8a2 - 7 commits from branch master
• 347ffbe1 - Use LabelSerializer in label controllers
• 63de56ae - Introduce AjaxCache class
• cef34d48 - Colorize labels in search field (!11047 (merged))

Compare with previous version

Checked out the branch and reviewed the UX. Everything looks and works as it should

Tests and code review are remaining. The rest should be finished.

mentioned in commit 7122debe

added 1 commit

• 7122debe - Colorize labels in search field (!11047 (merged))

Compare with previous version

@grzesiek I haven't looked into the label controller specs yet. Apart from that, the backend work should be finished. Could you already take a look at it please?

mentioned in commit 77a563ba

added 30 commits

• 7122debe...ecdf7c26 - 27 commits from branch master
• 6c954b7b - Use LabelSerializer in label controllers
• 5595d106 - Introduce AjaxCache class
• 77a563ba - Colorize labels in search field (!11047 (merged))

Compare with previous version

mentioned in commit 1997fa44

added 1 commit

• 1997fa44 - Colorize labels in search field (!11047 (merged))

Compare with previous version

mentioned in commit 91656a23

added 2 commits

• 1ea8324a - Introduce AjaxCache class
• 91656a23 - Colorize labels in search field (!11047 (merged))

Compare with previous version

mentioned in commit d73a2c86

added 1 commit

• d73a2c86 - Colorize labels in search field (!11047 (merged))

Compare with previous version

mentioned in commit 04805948

added 27 commits

• d73a2c86...91e8f770 - 24 commits from branch master
• 34c55909 - Use LabelSerializer in label controllers
• 7da159e7 - Introduce AjaxCache class
• 04805948 - Colorize labels in search field (!11047 (merged))

Compare with previous version

mentioned in commit 6aaa48ef

added 1 commit

• 6aaa48ef - Colorize labels in search field (!11047 (merged))

Compare with previous version

mentioned in commit 224ce397

added 1 commit

• 224ce397 - Colorize labels in search field (!11047 (merged))

Compare with previous version

mentioned in commit 76ea6ea0

added 10 commits

• 224ce397...79dc8171 - 7 commits from branch master
• 78094f5a - Use LabelSerializer in label controllers
• db5edcfc - Introduce AjaxCache class
• 76ea6ea0 - Colorize labels in search field (!11047 (merged))

Compare with previous version

Only now I found out how to reproduce https://gitlab.com/gitlab-org/gitlab-ce/issues/30468#note_27698422 and pasting multiple filters at once doesn't color the labels.

@victorwu Is that something we can cover in a subsequent merge request?

Is that something we can cover in a subsequent merge request?

If my opinion is worth anything, I think this is a good idea. 14 changed files with 429 additions and 7 deletions is plenty. :)

It looks like pasting doesn't always work anyway: #31859 (moved)

@jschatz1 Can you review please?

assigned to @jschatz1

unmarked as a Work In Progress

@winh I left just a few questions, it looks really good, nice work!

mentioned in commit 1d79ba48

@grzesiek Thank you for the review and all the help!

added 81 commits

• 76ea6ea0...8a5c95d2 - 78 commits from branch master
• 538ba76d - Use LabelSerializer in label controllers
• 120b1ea0 - Introduce AjaxCache class
• 1d79ba48 - Colorize labels in search field (!11047 (merged))

Compare with previous version

changed milestone to %9.2

resolved all discussions

@winh no need to call labels.json twice. Grab the data before.

assigned to @winh

@winh Some background for the approach you'll want for this with the current code you've provided...

---

The label dropdown is a droplab dropdown that performs a single ajax request using the Ajax plugin and then filters them using the Filter plugin.

You can see where the FilteredSearchDropdownManager maps this here: http://gitlab.com/gitlab-org/gitlab-ce/blob/c89849af47dc3ce788adbc7ef06af8a39a5347fd/app/assets/javascripts/filtered_search/filtered_search_dropdown_manager.js#L48-53

You'll see the gl property is set to DropdownNonUser, this refers to the dropdown class used by the label dropdown.

So, in the DropdownNonUser class, we can see we use the Ajax and Filter plugins. http://gitlab.com/gitlab-org/gitlab-ce/blob/c89849af47dc3ce788adbc7ef06af8a39a5347fd/app/assets/javascripts/filtered_search/dropdown_non_user.js#L45

You'll want to remove all references to the Ajax plugin, including is configuration. http://gitlab.com/gitlab-org/gitlab-ce/blob/c89849af47dc3ce788adbc7ef06af8a39a5347fd/app/assets/javascripts/filtered_search/dropdown_non_user.js#L13-22

Then, after DropdownNonUser.prototype.init does this.droplab.addHook, you'll want to do this.droplab.setData(theDataFromAjaxCache).

no need to call labels.json twice. Grab the data before.

@jschatz1 I agree, calling it twice is unfortunate. However changing that requires refactoring in the dropdown code which I wouldn't finish before feature freeze for %9.2. Do you want it to be moved to %9.3 then?

added 1 commit

• d3cd9c33 - temporary disable spec/javascripts/bootstrap_linked_tabs_spec.js

Compare with previous version

added 1 commit

• 91815ab4 - Use absolute URLs for fixtures

Compare with previous version

Sorry about the confusion—the last two pushes were not intended to go into this branch.

approved this merge request

mentioned in commit a4c90a41

merged

resolved all discussions

mentioned in issue #31897 (closed)

mentioned in merge request !11170 (merged)

mentioned in issue #36852