-
-
Notifications
You must be signed in to change notification settings - Fork 8.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Overhaul search with a Command Palette #7569
base: master
Are you sure you want to change the base?
Changes from all commits
1b37f3e
39589a3
22d83a7
cbc13a7
a240f54
654d71e
065e979
5048640
4e7f0c3
64864ab
a76919d
9b70f56
0fb9f1f
5bb9393
fb54dc0
1e2f45d
19077cd
f850297
31e345f
d7b4b92
2688ce3
b2f9a44
796395d
1d1df8e
bb9faf0
bc7da9e
0511461
081f60b
6f09f80
72252ec
8dc955d
285776f
51d2626
f44be2a
0a409e0
ce0410e
f203189
157858c
ad2e760
df50631
d61a966
1cf43ca
f608c94
0d8e1a2
480b919
5576ea2
1617295
1783029
accd74d
abd3361
ff66478
a52f773
aec5c9f
15e7c9a
c0ae7da
ec58717
231631d
3996dc0
5fa1ed0
e475abe
4235d82
4b37c45
f205df1
3a14aa9
7569e2e
f4ed718
c498d44
b95a175
6f73030
da84e04
63b4fd2
20a4205
be776a2
d232157
7f53092
ce802bc
6092031
8d295bd
bf76b9c
4639387
a51e660
886e25e
c22f697
74d8598
3c35b30
6086a0f
8e85f30
2a96790
57017e6
94f4b7f
ef34c0b
692944f
c87bc9a
fb1641f
27c778c
972df51
0274960
c63da6b
1eee9d2
f8a6f63
337c05c
63869db
b208d80
30f80e0
3f61445
7af9147
a50e353
8344087
cd638da
8814ca7
8bdea5a
5cebb05
1a85512
5d1587e
599b02f
1ebc4f5
5ab218b
c5d77ee
0982df5
1db14a2
ecbbc8a
0ea776e
0833a4a
97363b4
a20e66a
b0dd174
57905c2
cdff812
7855ffc
13d2021
40e1d6b
d446143
ba7400f
f4f567a
89da576
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -158,7 +158,7 @@ public void doSuggestOpenSearch(StaplerRequest2 req, StaplerResponse2 rsp, @Quer | |
public void doSuggest(StaplerRequest2 req, StaplerResponse2 rsp, @QueryParameter String query) throws IOException, ServletException { | ||
Result r = new Result(); | ||
for (SuggestedItem item : getSuggestions(req, query)) | ||
r.suggestions.add(new Item(item.getPath())); | ||
r.suggestions.add(new Item(item.getPath(), item.getUrl())); | ||
|
||
rsp.serveExposedBean(req, r, Flavor.JSON); | ||
} | ||
|
@@ -252,12 +252,25 @@ public static class Result { | |
|
||
@ExportedBean(defaultVisibility = 999) | ||
public static class Item { | ||
|
||
@Exported | ||
@SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD", justification = "read by Stapler") | ||
public String name; | ||
|
||
private final String url; | ||
|
||
public Item(String name) { | ||
this(name, null); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this result in correct behavior when used? I cannot tell. This also ends up bloating the responses of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Testing the autocomplete control with the Design Library and it works fine. It does bloat the response (with a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Probably subtype / separate types. Unclear why
Works for me. |
||
} | ||
|
||
public Item(String name, String url) { | ||
this.name = name; | ||
this.url = url; | ||
} | ||
|
||
@Exported | ||
public String getUrl() { | ||
return url; | ||
} | ||
} | ||
|
||
|
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
<!-- | ||
The MIT License | ||
|
||
Copyright (c) 2024, Jenkins contributors | ||
|
||
Permission is hereby granted, free of charge, to any person obtaining a copy | ||
of this software and associated documentation files (the "Software"), to deal | ||
in the Software without restriction, including without limitation the rights | ||
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell | ||
copies of the Software, and to permit persons to whom the Software is | ||
furnished to do so, subject to the following conditions: | ||
|
||
The above copyright notice and this permission notice shall be included in | ||
all copies or substantial portions of the Software. | ||
|
||
THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR | ||
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, | ||
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE | ||
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER | ||
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, | ||
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN | ||
THE SOFTWARE. | ||
--> | ||
|
||
<?jelly escape-by-default='true'?> | ||
<j:jelly xmlns:j="jelly:core" xmlns:s="jelly:stapler" xmlns:l="/lib/layout"> | ||
<s:documentation> | ||
The command palette overlay | ||
</s:documentation> | ||
|
||
<div id="command-palette-i18n" class="i18n" | ||
data-no-results-for="${%No results for}" | ||
data-help="${%Help}" | ||
data-get-help="${%Get help using Jenkins search}" | ||
/> | ||
|
||
<dialog id="command-palette" class="jenkins-command-palette__dialog"> | ||
<div class="jenkins-command-palette__wrapper"> | ||
<div class="jenkins-command-palette"> | ||
<l:search-bar id="command-bar" clazz="jenkins-command-palette__search" hasKeyboardShortcut="false" /> | ||
<div id="search-results-container" class="jenkins-command-palette__results-container"> | ||
<div id="search-results" class="jenkins-command-palette__results" /> | ||
</div> | ||
</div> | ||
</div> | ||
</dialog> | ||
</j:jelly> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,25 +1,16 @@ | ||
<?jelly escape-by-default='true'?> | ||
<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:l="/lib/layout"> | ||
<div class="searchbox hidden-xs"> | ||
<!-- search box --> | ||
<j:set var="searchURL" value="${h.searchURL}"/> | ||
<form action="${searchURL}" method="get" style="position:relative;" class="no-json" name="search" role="search"> | ||
<!-- this div is used to calculate the width of the text box --> | ||
<div id="search-box-sizer"/> | ||
<div id="searchform"> | ||
<input name="q" placeholder="${searchPlaceholder}" id="search-box" class="main-search__input" value="${request.getParameter('q')}" role="searchbox" /> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This gets dropped, but the legacy search pages and their internal links ( |
||
<j:jelly xmlns:j="jelly:core" xmlns:l="/lib/layout" xmlns:st="jelly:stapler"> | ||
<div class="page-header__hyperlinks"> | ||
<j:forEach var="factory" items="${h.searchFactories}"> | ||
<st:include it="${factory}" page="search.jelly" optional="true" /> | ||
</j:forEach> | ||
|
||
<span class="main-search__icon-leading"> | ||
<l:icon src="symbol-search"/> | ||
</span> | ||
<a href="${searchHelpUrl}" class="main-search__icon-trailing"> | ||
<l:icon src="symbol-help-circle"/> | ||
</a> | ||
|
||
<div id="search-box-completion" data-search-url="${searchURL}" /> | ||
<st:adjunct includes="jenkins.views.JenkinsHeader.search-box" /> | ||
timja marked this conversation as resolved.
Show resolved
Hide resolved
|
||
</div> | ||
</form> | ||
<button id="button-open-command-palette" tooltip="${%Search}" data-keyboard-shortcut="CMD+K" | ||
data-search-url="${rootURL + '/search/suggest'}" | ||
data-search-help-url="${searchHelpUrl}" | ||
> | ||
<l:icon src="symbol-search" /> | ||
</button> | ||
</div> | ||
<script src="${resURL}/jsbundles/keyboard-shortcuts.js" type="text/javascript"/> | ||
</j:jelly> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
/** | ||
* @param {string} searchTerm | ||
*/ | ||
function search(searchTerm) { | ||
const address = document.getElementById("button-open-command-palette").dataset | ||
.searchUrl; | ||
return fetch(`${address}?query=${encodeURIComponent(searchTerm)}`); | ||
} | ||
|
||
export default { search: search }; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
import { LinkResult } from "./models"; | ||
import Search from "@/api/search"; | ||
import * as Symbols from "./symbols"; | ||
|
||
export const JenkinsSearchSource = { | ||
execute(query) { | ||
const rootUrl = document.head.dataset.rooturl; | ||
|
||
function correctAddress(url) { | ||
if (url.startsWith("/")) { | ||
url = url.substring(1); | ||
} | ||
|
||
return rootUrl + "/" + url; | ||
} | ||
|
||
return Search.search(query).then((rsp) => | ||
rsp.json().then((data) => { | ||
return data["suggestions"].slice().map((e) => | ||
LinkResult({ | ||
icon: Symbols.SEARCH, | ||
label: e.name, | ||
url: correctAddress(e.url), | ||
}), | ||
); | ||
}), | ||
); | ||
}, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found the UX around these sorts of things a little confusing and offering multiple results for the same thing to be odd. Happy to restore it though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense to not have multiple items for the same thing (especially if one's just a substring). This removes all of them though?
OTOH There are other interesting actions (or adjacent views) that could make sense to add here, so if there's a larger plan to add more, that would be fine. Or if this just clutters up the search for jobs named "con", that would be pointless.