jenkins
jenkins copied to clipboard
Overhaul search with a Command Palette


This PR introduces the Command Palette concept to Jenkins, first discussed back in the day in the November 2021 UX SIG meeting. You've probably used something similar if you've used VS Code, Jenkins.io, GitHub, Spotify etc. They're handy overlays that allow you to quickly use your keyboard to navigate, be that searching for different pages or finding functions in menus.
This is an early implementation and as such doesn't have all the features I want it to have, such as grouped results, icons and richer details for results, all aimed at making the search experience in Jenkins easier to use. I'd also like for plugin developers to eventually be able to integrate with the Command Palette so they can provide their own rich results/actions, for example being able to change theme from the palette.
Testing done
- Clicking the search button shows the Command Palette
- CMD + K shows the Command Palette
- Searching for terms brings up identical results to the previous search bar
Proposed changelog entries
- Add Command Palette as replacement for search bar
Proposed upgrade guidelines
N/A
Submitter checklist
- [ ] The Jira issue, if it exists, is well-described.
- [ ] The changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developers, depending on the change) and are in the imperative mood (see examples).
- Fill in the Proposed upgrade guidelines section only if there are breaking changes or changes that may require extra steps from users during upgrade.
- [ ] There is automated testing or an explanation as to why this change has no tests.
- [ ] New public classes, fields, and methods are annotated with
@Restricted
or have@since TODO
Javadocs, as appropriate. - [ ] New deprecations are annotated with
@Deprecated(since = "TODO")
or@Deprecated(forRemoval = true, since = "TODO")
, if applicable. - [ ] New or substantially changed JavaScript is not defined inline and does not call
eval
to ease future introduction of Content Security Policy (CSP) directives (see documentation). - [ ] For dependency updates, there are links to external changelogs and, if possible, full differentials.
- [ ] For new APIs and extension points, there is a link to at least one consumer.
Desired reviewers
@jenkinsci/sig-ux
Maintainer checklist
Before the changes are marked as ready-for-merge
:
- [ ] There are at least two (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 is not blocking the change.
- [ ] Changelog entries in the pull request title and/or Proposed changelog entries are accurate, human-readable, and in the imperative mood.
- [ ] Proper changelog labels are set so that the changelog can be generated automatically.
- [ ] If the change needs additional upgrade steps from users, the
upgrade-guide-needed
label is set and there is a Proposed upgrade guidelines section in the pull request title (see 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).
Please take a moment and address the merge conflicts of your pull request. Thanks!
Please take a moment and address the merge conflicts of your pull request. Thanks!
one test failing
Please take a moment and address the merge conflicts of your pull request. Thanks!
There's been several tests commented out/removed and this is due to the JS behind the Command Palette not being supported by HTMLUnit.
Is that still the case with now HTMLUnit 3?
Please take a moment and address the merge conflicts of your pull request. Thanks!
Please take a moment and address the merge conflicts of your pull request. Thanks!
is this ready for re-review by the security team?
is this ready for re-review by the security team?
Yep :)
I've tested the PR locally, and the XSS is no longer present. Everything else looks fine from a security perspective
Please take a moment and address the merge conflicts of your pull request. Thanks!
With the current very narrow fix, the same problem occurs when searching for "Built-In Node", "manage", or "admin" (a user name), probably everything that's not a job.
How are you running the PR? I can't seem to reproduce this.
Should this be a draft? It doesn't seem ready yet.
👍
How are you running the PR? I can't seem to reproduce this.
docker run --rm -ti -p 8080:8080 -e ID=7569 jenkins/core-pr-tester
Steps to reproduce:
- Create a job "fs"
- Navigate to
/job/fs/
- Cmd-K
- "Built-In Node" ->
/job/fs/computer/(built-in)/
(404) - "manage" ->
/job/fs/manage/
(404) - "admin" ->
/job/fs/user/admin/
(404) - "All" ->
/job/fs
(instead of the root URL, for the default view, a secondary view is/job/fs/view/whatever/
, 404)
- "Built-In Node" ->
How are you running the PR? I can't seem to reproduce this.
docker run --rm -ti -p 8080:8080 -e ID=7569 jenkins/core-pr-tester
Steps to reproduce:
Create a job "fs"
Navigate to
/job/fs/
Cmd-K
- "Built-In Node" ->
/job/fs/computer/(built-in)/
(404)- "manage" ->
/job/fs/manage/
(404)- "admin" ->
/job/fs/user/admin/
(404)- "All" ->
/job/fs
(instead of the root URL, for the default view, a secondary view is/job/fs/view/whatever/
, 404)
Thanks! I can't run Docker on my machine at the moment due to some issues however I've tried my best to reproduce it by altering the Jenkins URL and updating any relative URLs.
They should all be sorted now.
Please take a moment and address the merge conflicts of your pull request. Thanks!
Please take a moment and address the merge conflicts of your pull request. Thanks!
Please take a moment and address the merge conflicts of your pull request. Thanks!
Please take a moment and address the merge conflicts of your pull request. Thanks!
Tests passing and ATH passing - https://github.com/jenkinsci/acceptance-test-harness/pull/1103
Please take a moment and address the merge conflicts of your pull request. Thanks!
To help this MR forward I've removed the keyboard shortcut tooltip - will restore it in a future MR.
Might be color blindness, but the highlight of the selected row is too subtle. It's basically not present at all. In particular with a single result, it's difficult to tell it is selected.
Agreed, increased the contrast there - let me know if that's better.
The
/search
URL remains functional, is this intentional?
I haven't made any changes to it - would be happy to remove it in a follow up if its no longer used/convert the page to use the command palette for backwards compatibility.
Typing multiple characters quickly results in the same request being sent multiple times, indicating a problem in JS.
![]()
Great spot - I had messed up debounce
in the same way I had in the build history MR 🤦♂️ Fixed it now
It would be nice if, when closing and reopening the popup, the previous input were selected, for easy deletion (easily deferred).
Done
Thanks for addressing some of my feedback, I'll try to take a look later.
Note that most of my line comments remain unaddressed (not even rejected), in case that is just an oversight.
Thanks for addressing some of my feedback, I'll try to take a look later.
Note that most of my line comments remain unaddressed (not even rejected), in case that is just an oversight.
Thanks! From what I can see I've responded to your line comments unless I'm missing some
There's a search bug that becomes slightly worse with this change. I think it's non-blocking, but noting in case you think different.
Create folders Plugins
, Plugins/tinfoil-scan
, and freestyle job Plugins/tinfoil-scan/PR-22
(I found this on ci.j.io trying to compare before/after). Search for plugin fo
. The first hit will be the non-existent job Plugins » PR-22 and selecting it will get a 404 Rage Jenkins. (The existing UI kept you in an infinite loop of search result pages linking to themselves, never jumping to a (non-existing) exact match.)
Is it deliberate that there is currently no indicator for the Cmd-K shortcut? Is that something an updated #8435 would add on top?
https://plugins.jenkins.io/lucene-search/ kinda breaks with this PR as the (now augmented) legacy result pages are no longer used. https://github.com/jenkinsci/lucene-search-plugin/blob/lucene-search/src/main/java/org/jenkinsci/plugins/lucene/search/FreeTextSearchFactory.java#L20 is how the plugin currently provides custom search result pages -- is there something we can tell the maintainer to adapt to this change? (This is the only issue that for me is still (semi-)blocking; if we can do something for this plugin without an extensive rewrite of this PR or that plugin, we should.)
Eventually I'd like the command palette and its results to be extensible by developers (and maybe that's something that plugin could implement in the future).
Could a workaround for the timebeing for the plugin to add an additional control to the header/somewhere to access that search page?
Could a workaround for the timebeing for the plugin to add an additional control to the header/somewhere to access that search page?
I think something like that would be reasonable, if it can be implemented in a way that doesn't break regularly whenever the header is changed. Even better, ability to remove the new search, and restore the old text field via Jelly somehow?
Alternatively, an extension point that overrides where you go for a search result from the popup? By default, directly there, and the plugin could override it with its fancy results page?
Please take a moment and address the merge conflicts of your pull request. Thanks!
Hey.
Let me know if this is the right approach.. I've added support for plugins to add their own search buttons to the header, e.g. in this example Lucene has added a 'Lucene' button to access their UI by implementing search.jelly
. It's fairly minimal to implement and shouldn't be too much work for the plugin owners to shift the search input to their page.
<?jelly escape-by-default='true'?>
<j:jelly xmlns:j="jelly:core">
<a href="...">
Lucene
</a>
</j:jelly>