[JENKINS-62024] Add quick filter box for dropdown lists in hetero-list #4688

johnlinp · 2020-04-26T18:07:41Z

See JENKINS-62024.

Proposed changelog entries

Add quick filter box for menus created by buttons.

Proposed upgrade guidelines

N/A

Submitter checklist

(If applicable) Jira issue is well described
Changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developer, depending on the change). Examples
- Fill-in the Proposed changelog entries section only if there are breaking changes or other changes which may require extra steps from users during the upgrade
Appropriate autotests or explanation to why this change has no tests
For dependency updates: links to external changelogs and, if possible, full diffs

Desired reviewers

@mention Oleg Nenashev

Maintainer checklist

Before the changes are marked as ready-for-merge:

There are at least 2 approvals for the pull request and no outstanding requests for change
Conversations in the pull request are over OR it is explicit that a reviewer does not block the change
Changelog entries in the PR title and/or Proposed changelog entries are correct
Proper changelog labels are set so that the changelog can be generated automatically
If the change needs additional upgrade steps from users, upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the PR title. (example)
If it would make sense to backport the change to LTS, a Jira issue must exist, be a Bug or Improvement, and be labeled as lts-candidate to be considered (see query).

daniel-beck · 2020-04-26T19:16:22Z

Reviewer note: This change will not be reflected in the PR tester by default.

fqueiruga · 2020-04-27T07:20:34Z

I don't think modifying the YUI code is a good approach. It's library code that's been stable for 12+ years, so I'd be very wary when making the decision to touch it.

The DragDrop widget also builds on the YUI code, but does so by decorating it: https://github.com/jenkinsci/jenkins/blob/master/core/src/main/resources/lib/form/dragdrop/dragdrop.js. This, if possible, is the approach I suggest. Otherwise I'd just build a new dropdown widget altogether.

daniel-beck · 2020-04-27T12:07:41Z

I don't think modifying the YUI code is a good approach.

We've done it before 🤷 It's not like the library is pristine.

fqueiruga · 2020-04-29T07:34:17Z

Still, I'd suggest trying the other approach first.

johnlinp · 2020-04-29T07:42:09Z

@fqueiruga I was looking for an example of decorations on YUI but failed. Thank you for providing the DragDrop example. I'll work on it.

johnlinp · 2020-05-01T04:15:32Z

Hi @fqueiruga,

I pushed a new commit that wraps around YAHOO.widget.Button and apply the quick filter box on it. Please take a look at it if it's acceptable. Thank you.

varyvol · 2020-06-04T09:16:29Z

@johnlinp I don't feel confident enough (with my UI knowledge) to approve, but it looks good. Maybe you could add some explanatory comments to the code and some screenshots where the functionality can be observed.

@fqueiruga could you maybe have a look? The current status uses your preferred approach.

johnlinp · 2020-07-22T15:35:20Z

Hi @fqueiruga,

Would you please review my PR? Thanks!

fqueiruga · 2020-07-23T14:33:00Z

I think the approach is good even though the styling is a bit rough, needs some paddings :D.

Did you consoider only applying the filter when the number of options is over 5 or something? There are many hetero lists with only 2 options and that may be overkill.

johnlinp · 2020-07-25T01:57:41Z

Hi @fqueiruga,

Thank you for the review. Yes, I think it makes sense to only apply the filter when the number of options is at least 5. I'll push a new commit to implement this.

alecharp · 2022-03-15T14:43:19Z

@johnlinp is this still something we are attached to? do you have time to work on it or should we close this PR?

johnlinp · 2022-03-15T20:38:21Z

@alecharp Yes, I would love to continue working on it. Let's see if the reviewers have any comments. Thanks.

janfaracik · 2022-03-15T22:06:03Z

Works well 👍 UI is a touch rough but so is YUI in general.

I want to update/replace the YUI menus eventually but that's no reason to delay this enhancement.

Only thing I'd say is see if there's a way of disabling autocomplete/autocorrect on the filter field.

johnlinp · 2022-03-16T15:39:29Z

Thanks @janfaracik! I'll try to disable autocomplete and autocorrect for this field.

alecharp · 2022-03-16T16:07:46Z

@johnlinp this is great. Could you work on the merged conflicts so we can move this PR forward. Thanks.

johnlinp · 2022-03-17T04:48:58Z

@alecharp I've updated the branch to resolve the conflict. Thanks.

@janfaracik I didn't see autocomplete or autocorrect on my end. Can you provide the reproduction steps to show autocomplete or autocorrect for the filter? Thank you!

alecharp · 2022-03-30T13:35:13Z

@janfaracik I saw in a PR that you are removing the YUI tooltip implementation. Is there an initiative to remove YUI completely from the core? The work here from @johnlinp seems to be good and could be integrated, no? WDYT @fqueiruga ?

janfaracik · 2022-03-30T20:56:06Z

core/src/main/resources/lib/form/filter-menu-button/filter-menu-button.js

+function _createFilter (menu) {
+    var filterInput = document.createElement("input");
+    filterInput.style.width = '100%';
+    filterInput.setAttribute("placeholder", "Filter");


Seems like the spellcheck might just be a macOS/Safari issue.

Suggested change

filterInput.setAttribute("placeholder", "Filter");

filterInput.setAttribute("placeholder", "Filter");

filterInput.setAttribute("spellcheck", "false");

filterInput.setAttribute("type", "search");

This fixed the spellcheck problem for me, also uses the correct input type so you get a nice little ⓧ button for clearing text 👍

janfaracik · 2022-03-30T20:57:26Z

@janfaracik I saw in a PR that you are removing the YUI tooltip implementation. Is there an initiative to remove YUI completely from the core? The work here from @johnlinp seems to be good and could be integrated, no? WDYT @fqueiruga ?

Hoping to remove as much of the YUI stuff as possible eventually, might take a good while though just due to how much there is. This work is definitely good though and should be integrated 👍

fqueiruga · 2022-03-31T07:41:12Z

core/src/main/resources/lib/form/filter-menu-button/filter-menu-button.js

@@ -0,0 +1,56 @@
+function createFilterMenuButton (button, menu, menuAlignment, menuMinScrollHeight) {


If these functions are to be global I'd suggest maybe namespacing them. I can see createFilterMenuButton being a name that could be easily overriden.

it's already quite a long name, I think it will be okay for now anyway. Unless you've got any specific suggestions?

fqueiruga · 2022-03-31T07:43:19Z

My comments about holding off removing YUI are 2 years old and were of a time before @janfaracik 's amazing work. I'd say that, if possible, it may be beneficial to include this in YUI removal efforts.

basil · 2022-04-10T17:43:27Z

I am getting a generally positive assessment of this PR from reviewers, but on the other hand none of them have approved the PR, which makes me wonder why. Reviewers should consider providing an Apache-style vote (+1, 0, -1, and fractions) to make their assessment explicit. If the general consensus is strongly positive (e.g., explicit GitHub approvals or +1 votes), I will merge the PR. In contrast, if the general consensus is strongly negative (which I do not believe is the case for this PR!) I would close the PR. If, on the other hand, the general consensus is closer toward neutral, it would be helpful for reviewers to explicitly state which action items are needed in order to move the PR closer to the positive side of the voting number line.

timja · 2022-04-14T22:16:16Z

I like the change, 2 areas I think we can improve before I'll give a +1

Focus state to match other components, e.g.:

Text should be aligned to other text not touching the left edge

timja · 2022-04-17T08:32:27Z

core/src/main/resources/lib/form/filter-menu-button/filter-menu-button.js

+  filterInput.setAttribute("spellcheck", "false");
+  filterInput.setAttribute("type", "search");
+
+  filterInput.addEventListener('input', (event) => _applyFilterKeyword(menu, event));


changed to input from keyup so that the search clear event works

timja · 2022-04-17T08:35:10Z

Pushed a commit to address my concerns. (also reformatted it to match editorconfig file)

NotMyFault

Looks ok to me, this component (likely) needs to be revisited in Jan's anyway, to align it with the overhauled look of the project configuration view.

Though to note, various tests are failing.

timja · 2022-04-17T14:31:18Z

I assume that was an issue with optional chaining and html unit =/

timja · 2022-04-17T21:25:02Z

This PR is now ready for merge, after ~24 hours, we will merge it if there's no negative feedback.

Thanks!

johnlinp · 2022-04-18T14:51:22Z

Thank you @timja for reviewing my code and updating my branch!

MarkEWaite added the work-in-progress The PR is under active development, not ready to the final review label Apr 26, 2020

johnlinp force-pushed the add-dropdown-filter branch from ea5506f to d2dc93b Compare May 1, 2020 04:12

johnlinp changed the title ~~[JENKINS-62024] PoC: Add quick filter box for dropdown lists~~ [JENKINS-62024] Add quick filter box for dropdown lists in hetero-list May 1, 2020

johnlinp force-pushed the add-dropdown-filter branch from d2dc93b to 4d53f5f Compare May 1, 2020 04:21

fcojfernandez requested a review from fqueiruga June 4, 2020 09:16

johnlinp force-pushed the add-dropdown-filter branch from 4d53f5f to f85ca83 Compare July 22, 2020 07:09

johnlinp force-pushed the add-dropdown-filter branch from ba67395 to afa1d5d Compare July 25, 2020 02:05

ianwilliams1 mentioned this pull request Dec 5, 2020

[FEATURE] Improve the usability of the actual value selection process jenkinsci/rest-list-parameter-plugin#22

Closed

alecharp added the unresolved-merge-conflict There is a merge conflict with the target branch. label Mar 15, 2022

daniel-beck requested review from janfaracik and daniel-beck March 15, 2022 15:27

johnlinp added 2 commits March 16, 2022 15:54

[JENKINS-62024] Add quick filter box for dropdown lists in hetero-list

6947ae5

[JENKINS-62024] Only apply the filter when there are at least 5 options

b572a67

johnlinp force-pushed the add-dropdown-filter branch from afa1d5d to b572a67 Compare March 17, 2022 04:46

alecharp added needs-more-reviews Complex change, which would benefit from more eyes and removed unresolved-merge-conflict There is a merge conflict with the target branch. labels Mar 18, 2022

alecharp removed the work-in-progress The PR is under active development, not ready to the final review label Mar 29, 2022

janfaracik reviewed Mar 30, 2022

View reviewed changes

fqueiruga reviewed Mar 31, 2022

View reviewed changes

basil requested a review from fqueiruga April 10, 2022 17:44

Merge branch 'master' into add-dropdown-filter

5d1fbf4

timja added rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted and removed needs-more-reviews Complex change, which would benefit from more eyes labels Apr 14, 2022

Address review comments

2c0510d

timja reviewed Apr 17, 2022

View reviewed changes

timja approved these changes Apr 17, 2022

View reviewed changes

timja requested review from janfaracik and a team April 17, 2022 08:34

NotMyFault approved these changes Apr 17, 2022

View reviewed changes

Syntax that htmlunit can understand -.-

dc6b0f7

timja added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Apr 17, 2022

timja merged commit b3b059f into jenkinsci:master Apr 19, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[JENKINS-62024] Add quick filter box for dropdown lists in hetero-list #4688

[JENKINS-62024] Add quick filter box for dropdown lists in hetero-list #4688

johnlinp commented Apr 26, 2020 •

edited by MarkEWaite

Loading

daniel-beck commented Apr 26, 2020

fqueiruga commented Apr 27, 2020 •

edited

Loading

daniel-beck commented Apr 27, 2020

fqueiruga commented Apr 29, 2020

johnlinp commented Apr 29, 2020

johnlinp commented May 1, 2020

varyvol commented Jun 4, 2020

johnlinp commented Jul 22, 2020

fqueiruga commented Jul 23, 2020 •

edited

Loading

johnlinp commented Jul 25, 2020 •

edited

Loading

alecharp commented Mar 15, 2022

johnlinp commented Mar 15, 2022

janfaracik commented Mar 15, 2022

johnlinp commented Mar 16, 2022

alecharp commented Mar 16, 2022

johnlinp commented Mar 17, 2022

alecharp commented Mar 30, 2022

janfaracik Mar 30, 2022 •

edited

Loading

janfaracik commented Mar 30, 2022

fqueiruga Mar 31, 2022

timja Apr 17, 2022

fqueiruga commented Mar 31, 2022

basil commented Apr 10, 2022

timja commented Apr 14, 2022

timja Apr 17, 2022

timja commented Apr 17, 2022

NotMyFault left a comment •

edited

Loading

timja commented Apr 17, 2022

timja commented Apr 17, 2022

johnlinp commented Apr 18, 2022

		@@ -0,0 +1,56 @@
		function createFilterMenuButton (button, menu, menuAlignment, menuMinScrollHeight) {

[JENKINS-62024] Add quick filter box for dropdown lists in hetero-list #4688

[JENKINS-62024] Add quick filter box for dropdown lists in hetero-list #4688

Conversation

johnlinp commented Apr 26, 2020 • edited by MarkEWaite Loading

Proposed changelog entries

Proposed upgrade guidelines

Submitter checklist

Desired reviewers

Maintainer checklist

daniel-beck commented Apr 26, 2020

fqueiruga commented Apr 27, 2020 • edited Loading

daniel-beck commented Apr 27, 2020

fqueiruga commented Apr 29, 2020

johnlinp commented Apr 29, 2020

johnlinp commented May 1, 2020

varyvol commented Jun 4, 2020

johnlinp commented Jul 22, 2020

fqueiruga commented Jul 23, 2020 • edited Loading

johnlinp commented Jul 25, 2020 • edited Loading

alecharp commented Mar 15, 2022

johnlinp commented Mar 15, 2022

janfaracik commented Mar 15, 2022

johnlinp commented Mar 16, 2022

alecharp commented Mar 16, 2022

johnlinp commented Mar 17, 2022

alecharp commented Mar 30, 2022

janfaracik Mar 30, 2022 • edited Loading

Choose a reason for hiding this comment

janfaracik commented Mar 30, 2022

fqueiruga Mar 31, 2022

Choose a reason for hiding this comment

timja Apr 17, 2022

Choose a reason for hiding this comment

fqueiruga commented Mar 31, 2022

basil commented Apr 10, 2022

timja commented Apr 14, 2022

timja Apr 17, 2022

Choose a reason for hiding this comment

timja commented Apr 17, 2022

NotMyFault left a comment • edited Loading

Choose a reason for hiding this comment

timja commented Apr 17, 2022

timja commented Apr 17, 2022

johnlinp commented Apr 18, 2022

johnlinp commented Apr 26, 2020 •

edited by MarkEWaite

Loading

fqueiruga commented Apr 27, 2020 •

edited

Loading

fqueiruga commented Jul 23, 2020 •

edited

Loading

johnlinp commented Jul 25, 2020 •

edited

Loading

janfaracik Mar 30, 2022 •

edited

Loading

NotMyFault left a comment •

edited

Loading