Skip to content
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

Add search bar component #7074

Merged
merged 11 commits into from
Sep 16, 2022
Merged

Conversation

janfaracik
Copy link
Contributor

@janfaracik janfaracik commented Sep 6, 2022

This PR standardises the search bar introduced in #5692 and the newly introduced large variant in #6783 as a new component so it can be reused across Jenkins/plugins.

It's received a small revamp for consistency + fixes some visual bugs:

Screenshot 2022-09-06 at 20 12 21

Screenshot 2022-09-06 at 20 12 49

It also now has a keyboard shortcut, tap '/' on your keyboard to focus it (plus the option to disable that if you have multiple search inputs on one page).

Proposed changelog entries

  • Press '/' to focus the search bar on a page.

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) and are in the imperative mood. 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
  • New public classes, fields, and methods are annotated with @Restricted or have @since TODO Javadoc, 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 directives (see documentation on jenkins.io).
  • For dependency updates: links to external changelogs and, if possible, full diffs

Desired reviewers

@jenkinsci/sig-ux

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 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, 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).

commit fa24b35
Author: Jan Faracik <[email protected]>
Date:   Tue Sep 6 20:30:50 2022 +0100

    Add autofocus

commit 2b2d85b
Author: Jan Faracik <[email protected]>
Date:   Tue Sep 6 15:00:35 2022 +0100

    Update plugin-manager-ui.js

commit 153f744
Author: Jan Faracik <[email protected]>
Date:   Tue Sep 6 14:53:05 2022 +0100

    Linting

commit 4ddad3d
Author: Jan Faracik <[email protected]>
Date:   Tue Sep 6 14:51:53 2022 +0100

    Reset

commit 377df27
Author: Jan Faracik <[email protected]>
Date:   Tue Sep 6 14:50:36 2022 +0100

    Use new search bar

commit a54d42b
Merge: f6daa76 6db88c3
Author: Jan Faracik <[email protected]>
Date:   Tue Sep 6 14:49:25 2022 +0100

    Merge branch 'master' into new-search-bar-component

commit f6daa76
Author: Jan Faracik <[email protected]>
Date:   Wed Aug 10 19:27:12 2022 +0100

    Update search-bar.jelly

commit 9bec29f
Author: Jan Faracik <[email protected]>
Date:   Wed Aug 10 19:26:18 2022 +0100

    Update search-bar.jelly

commit afebc6f
Author: Jan Faracik <[email protected]>
Date:   Wed Aug 10 19:24:51 2022 +0100

    Update search-bar.less

commit f97ff09
Author: Jan Faracik <[email protected]>
Date:   Wed Aug 10 19:19:32 2022 +0100

    Update search-bar.less

commit 77e62b8
Author: Jan Faracik <[email protected]>
Date:   Tue Aug 9 22:02:13 2022 +0100

    Linting

commit 60d6a7f
Merge: 591e983 f5d7165
Author: Jan Faracik <[email protected]>
Date:   Tue Aug 9 21:56:52 2022 +0100

    Merge branch 'master' into new-search-bar-component

commit 591e983
Merge: 6cfce9f ef7f408
Author: Jan Faracik <[email protected]>
Date:   Mon Jul 25 21:04:10 2022 +0100

    Merge branch 'master' into new-search-bar-component

commit 6cfce9f
Author: Jan Faracik <[email protected]>
Date:   Thu Jul 21 22:41:27 2022 +0100

    Replace plugin manager search with new search component

commit 95922de
Merge: 65f7cd3 09eb5f1
Author: Jan Faracik <[email protected]>
Date:   Thu Jul 21 22:32:05 2022 +0100

    Merge branch 'new-plugin-manager-1' into new-search-bar-component

commit 65f7cd3
Author: Jan Faracik <[email protected]>
Date:   Thu Jul 21 22:31:54 2022 +0100

    Add autofocus

commit 1db67ab
Author: Jan Faracik <[email protected]>
Date:   Thu Jul 21 18:06:20 2022 +0100

    Update Jenkins search bar style

commit 83e4b12
Author: Jan Faracik <[email protected]>
Date:   Thu Jul 21 17:16:57 2022 +0100

    Add keyboard shortcut symbol

commit 1f5c74e
Merge: 9878cac 6f7a259
Author: Jan Faracik <[email protected]>
Date:   Thu Jul 21 16:31:30 2022 +0100

    Merge branch 'add-search-keyboard-shortcut' into new-search-bar-component

commit 6f7a259
Author: Jan Faracik <[email protected]>
Date:   Thu Jul 21 16:31:09 2022 +0100

    Update keyboard-shortcuts.js

commit 9b10a2d
Author: Jan Faracik <[email protected]>
Date:   Thu Jul 21 15:33:21 2022 +0100

    Update yarn.lock

commit fac0b6f
Merge: 9654bfb bcefcc5
Author: Jan Faracik <[email protected]>
Date:   Thu Jul 21 14:56:29 2022 +0100

    Merge branch 'master' into add-search-keyboard-shortcut

commit 9654bfb
Author: Jan Faracik <[email protected]>
Date:   Thu Jul 21 14:51:01 2022 +0100

    Update

commit f2a6e56
Author: Jan Faracik <[email protected]>
Date:   Wed Jul 20 22:31:38 2022 +0100

    Add iPhone to use CMD key check

commit ce21573
Merge: cb34850 588ccd6
Author: Jan Faracik <[email protected]>
Date:   Wed Jul 20 21:39:18 2022 +0100

    Merge branch 'master' into add-search-keyboard-shortcut

commit cb34850
Merge: db5612a 237b9ca
Author: Jan Faracik <[email protected]>
Date:   Wed Jul 20 21:34:32 2022 +0100

    Merge branch 'master' into add-search-keyboard-shortcut

commit 9878cac
Author: Jan Faracik <[email protected]>
Date:   Wed Jul 20 21:24:27 2022 +0100

    Update search-bar.jelly

commit 930166b
Merge: d584d0f 237b9ca
Author: Jan Faracik <[email protected]>
Date:   Wed Jul 20 19:12:44 2022 +0100

    Merge branch 'master' into new-search-bar-component

commit 09eb5f1
Merge: b8880ed 7509a69
Author: Jan Faracik <[email protected]>
Date:   Mon Jul 18 21:58:09 2022 +0100

    Merge branch 'master' into new-plugin-manager-1

commit b8880ed
Author: Jan Faracik <[email protected]>
Date:   Mon Jul 18 14:55:39 2022 +0100

    Fix checkbox controller selection

commit 0b808b0
Merge: 4d640b4 38d4e86
Author: Jan Faracik <[email protected]>
Date:   Mon Jul 18 14:50:01 2022 +0100

    Merge branch 'master' into new-plugin-manager-1

commit 4d640b4
Merge: e14b7d2 f52d99a
Author: Tim Jacomb <[email protected]>
Date:   Thu Jul 14 09:01:42 2022 +0100

    Merge branch 'master' into new-plugin-manager-1

commit d584d0f
Author: Jan Faracik <[email protected]>
Date:   Wed Jul 13 23:38:19 2022 +0100

    Init

commit e14b7d2
Merge: 0f2ef2f a990ebc
Author: Jan Faracik <[email protected]>
Date:   Tue Jul 12 23:24:54 2022 +0100

    Merge branch 'master' into new-plugin-manager-1

commit db5612a
Author: Jan Faracik <[email protected]>
Date:   Tue Jul 12 22:12:00 2022 +0100

    Squashed commit of the following:

    commit b775940
    Author: Jan Faracik <[email protected]>
    Date:   Tue Jul 12 22:10:40 2022 +0100

        Update package

    commit 8191583
    Author: Jan Faracik <[email protected]>
    Date:   Tue Jul 12 22:04:31 2022 +0100

        Update headerContent.jelly

    commit 0f9f7c6
    Author: Jan Faracik <[email protected]>
    Date:   Tue Jul 12 22:03:37 2022 +0100

        Remove core-js dependency

    commit c6bb510
    Author: Jan Faracik <[email protected]>
    Date:   Tue Jul 12 21:55:29 2022 +0100

        Minimize code just for search bar

    commit 47ccdfb
    Author: Jan Faracik <[email protected]>
    Date:   Tue Jul 12 21:37:27 2022 +0100

        Reset more files

    commit b710e4d
    Author: Jan Faracik <[email protected]>
    Date:   Tue Jul 12 21:36:23 2022 +0100

        Reset

    commit 3da7082
    Author: Jan Faracik <[email protected]>
    Date:   Tue Jul 12 21:31:30 2022 +0100

        Add console warning for duplicated registrations

    commit 4dbc2e3
    Merge: 413eff2 76481f4
    Author: Jan Faracik <[email protected]>
    Date:   Tue Jul 12 16:37:16 2022 +0100

        Merge branch 'master' into keyboard-shortcuts

    commit 413eff2
    Merge: 80bee75 77a36fc
    Author: Jan Faracik <[email protected]>
    Date:   Fri Jun 3 23:10:07 2022 +0100

        Merge branch 'master' into keyboard-shortcuts

    commit 80bee75
    Author: Jan Faracik <[email protected]>
    Date:   Fri May 13 22:24:06 2022 +0100

        Update styles.less

    commit 875c93e
    Author: Jan Faracik <[email protected]>
    Date:   Fri May 13 22:23:34 2022 +0100

        Update keyboard-shortcuts.less

    commit 1b6391c
    Author: Jan Faracik <[email protected]>
    Date:   Fri May 13 22:17:13 2022 +0100

        Update keyboard-shortcuts.js

    commit c828d87
    Author: Jan Faracik <[email protected]>
    Date:   Fri May 13 22:16:57 2022 +0100

        Update keyboard-shortcuts.js

    commit 7ad4b6f
    Author: Jan Faracik <[email protected]>
    Date:   Fri May 13 22:14:47 2022 +0100

        Updated comments, remove unneeded dependency

    commit 9d7abbb
    Author: Jan Faracik <[email protected]>
    Date:   Fri May 13 22:04:07 2022 +0100

        Cleanup

    commit 76702be
    Author: Jan Faracik <[email protected]>
    Date:   Fri May 13 22:02:02 2022 +0100

        Update yarn.lock

    commit ea0e439
    Merge: 8d1112d e128402
    Author: Jan Faracik <[email protected]>
    Date:   Fri May 13 22:01:17 2022 +0100

        Merge branch 'master' into keyboard-shortcuts

    commit 8d1112d
    Author: Jan Faracik <[email protected]>
    Date:   Fri May 13 22:01:05 2022 +0100

        Isolate out tooltip changes

    commit 76fc089
    Merge: 7d7cd05 bbd7ca2
    Author: Jan Faracik <[email protected]>
    Date:   Fri May 13 21:42:05 2022 +0100

        Merge branch 'master' into keyboard-shortcuts

    commit 7d7cd05
    Author: Jan Faracik <[email protected]>
    Date:   Fri May 13 21:41:42 2022 +0100

        Update buildHealth.jelly

    commit 7ed6ff0
    Merge: 7bce4da c350527
    Author: Jan Faracik <[email protected]>
    Date:   Thu Apr 28 20:21:45 2022 +0100

        Merge branch 'master' into keyboard-shortcuts

    commit 7bce4da
    Author: Jan Faracik <[email protected]>
    Date:   Sat Apr 16 10:59:03 2022 +0100

        Add more shortcuts, add animation on key press

    commit 07b741b
    Author: Jan Faracik <[email protected]>
    Date:   Sat Apr 16 01:12:22 2022 +0100

        Add animation for selected key

    commit ce644cb
    Author: Jan Faracik <[email protected]>
    Date:   Sat Apr 16 00:47:45 2022 +0100

        Add keyboard shortcuts

    commit 34d6c89
    Merge: dd5b905 4a38d65
    Author: Jan Faracik <[email protected]>
    Date:   Sat Apr 16 00:40:41 2022 +0100

        Merge branch 'master' into keyboard-shortcuts

    commit dd5b905
    Author: Jan Faracik <[email protected]>
    Date:   Tue Mar 29 01:13:57 2022 +0100

        Add jelly component

    commit 959f586
    Merge: 54b47b5 32d790a
    Author: Jan Faracik <[email protected]>
    Date:   Mon Mar 28 23:51:43 2022 +0100

        Merge branch 'tippy-1-tooltip' into keyboard-shortcuts

    commit 54b47b5
    Author: Jan Faracik <[email protected]>
    Date:   Mon Mar 28 23:47:13 2022 +0100

        Reset further

    commit b580312
    Author: Jan Faracik <[email protected]>
    Date:   Mon Mar 28 23:46:01 2022 +0100

        Reset to master

    commit 32d790a
    Author: Jan Faracik <[email protected]>
    Date:   Mon Mar 28 14:06:21 2022 +0100

        Now possible to reregister tooltips for a specific container

    commit d5608a1
    Author: Jan Faracik <[email protected]>
    Date:   Mon Mar 28 13:16:30 2022 +0100

        Fix new line in queue tooltip

    commit f7659aa
    Author: Jan Faracik <[email protected]>
    Date:   Sun Mar 27 23:34:25 2022 +0100

        Fix last build successful from having a title

    commit bfce09a
    Author: Jan Faracik <[email protected]>
    Date:   Sun Mar 27 23:31:31 2022 +0100

        Fix new lines in tooltips, fix empty tooltips showing

    commit c58eb41
    Author: Jan Faracik <[email protected]>
    Date:   Fri Mar 25 14:27:13 2022 +0000

        Update keyboard shortcut logic

    commit b330661
    Author: Jan Faracik <[email protected]>
    Date:   Thu Mar 24 23:15:30 2022 +0000

        Initial commit, working search overlay

    commit 93632d3
    Merge: 60e4232 5144035
    Author: Jan Faracik <[email protected]>
    Date:   Tue Mar 22 09:52:43 2022 +0000

        Merge branch 'master' into tippy-1-tooltip

    commit 60e4232
    Author: Jan Faracik <[email protected]>
    Date:   Fri Mar 18 00:31:41 2022 +0000

        Update tooltips.js

    commit 9be65d6
    Author: Jan Faracik <[email protected]>
    Date:   Fri Mar 18 00:30:14 2022 +0000

        Update prototype.js

    commit 22f300a
    Merge: 7223890 422efe9
    Author: Jan Faracik <[email protected]>
    Date:   Thu Mar 17 20:40:32 2022 +0000

        Merge branch 'master' into tippy-1-tooltip

    commit 7223890
    Author: Jan Faracik <[email protected]>
    Date:   Tue Mar 15 13:28:35 2022 +0000

        Add percentage symbol

    commit 133c486
    Author: Jan Faracik <[email protected]>
    Date:   Tue Mar 15 09:51:35 2022 +0000

        Update header tooltips, add base tooltip styling

    commit 7e43dfa
    Author: Jan Faracik <[email protected]>
    Date:   Tue Mar 15 01:27:16 2022 +0000

        Update buildHealth.jelly

    commit df6847a
    Author: Jan Faracik <[email protected]>
    Date:   Tue Mar 15 01:25:16 2022 +0000

        Now possible to reregister tooltips

    commit 7e6c576
    Author: Jan Faracik <[email protected]>
    Date:   Tue Mar 15 01:15:10 2022 +0000

        Fix tooltip showing when it shouldn't, improve appearance

    commit 940d51a
    Author: Jan Faracik <[email protected]>
    Date:   Tue Mar 15 00:47:18 2022 +0000

        WB

    commit 1fbcc55
    Author: Jan Faracik <[email protected]>
    Date:   Tue Mar 15 00:38:20 2022 +0000

        Update tooltips.less

    commit 491b1f1
    Author: Jan Faracik <[email protected]>
    Date:   Tue Mar 15 00:36:07 2022 +0000

        Update hudson-behavior.js

    commit 09e9109
    Author: Jan Faracik <[email protected]>
    Date:   Tue Mar 15 00:33:13 2022 +0000

        WB

    commit db1928d
    Author: Jan Faracik <[email protected]>
    Date:   Tue Mar 15 00:24:10 2022 +0000

        Add initial JS files

    commit 0ff896b
    Author: Jan Faracik <[email protected]>
    Date:   Tue Mar 15 00:20:38 2022 +0000

        Add yarn deps

commit 0f2ef2f
Merge: db677f3 22dcefc
Author: Jan Faracik <[email protected]>
Date:   Thu Jul 7 13:48:53 2022 +0100

    Merge branch 'master' into new-plugin-manager-1

commit db677f3
Merge: 6e64c68 644a261
Author: Jan Faracik <[email protected]>
Date:   Wed Jul 6 13:37:57 2022 +0100

    Merge branch 'master' into new-plugin-manager-1

commit 6e64c68
Merge: dbc96f4 250540e
Author: Tim Jacomb <[email protected]>
Date:   Wed Jul 6 07:48:54 2022 +0100

    Merge branch 'master' into new-plugin-manager-1

commit dbc96f4
Merge: d74aeae 2b83c2e
Author: Tim Jacomb <[email protected]>
Date:   Wed Jul 6 06:43:36 2022 +0100

    Merge branch 'master' into new-plugin-manager-1

commit d74aeae
Author: Jan Faracik <[email protected]>
Date:   Tue Jul 5 23:29:13 2022 +0100

    Remove unused page variable

commit 9f3c69c
Merge: a46beb7 4ba9c08
Author: Jan Faracik <[email protected]>
Date:   Tue Jul 5 23:10:31 2022 +0100

    Merge branch 'master' into new-plugin-manager-1

commit a46beb7
Author: Jan Faracik <[email protected]>
Date:   Tue Jul 5 17:39:32 2022 +0100

    Improve visibility of search bar

commit a688b3b
Author: Jan Faracik <[email protected]>
Date:   Tue Jul 5 17:11:11 2022 +0100

    Restore tab bar

commit a39862c
Merge: 87c27c2 76e5c98
Author: Jan Faracik <[email protected]>
Date:   Tue Jul 5 17:08:02 2022 +0100

    Merge branch 'master' into new-plugin-manager-1

commit 87c27c2
Author: Jan Faracik <[email protected]>
Date:   Mon Jun 20 11:42:19 2022 +0100

    Update side-panel-tasks.less

commit abcbecf
Author: Jan Faracik <[email protected]>
Date:   Sun Jun 19 22:42:50 2022 +0100

    Update search bar

commit 88b2a59
Merge: f89fe63 2391319
Author: Jan Faracik <[email protected]>
Date:   Sun Jun 19 22:37:53 2022 +0100

    Merge branch 'move-manage-jenkins-less' into new-plugin-manager-1

commit 2391319
Merge: c9e2823 ac66b47
Author: Jan Faracik <[email protected]>
Date:   Thu Jun 16 17:47:45 2022 +0100

    Merge branch 'master' into move-manage-jenkins-less

commit c9e2823
Author: Jan Faracik <[email protected]>
Date:   Thu Jun 16 17:35:37 2022 +0100

    Update colors

commit c78dfd6
Author: Jan Faracik <[email protected]>
Date:   Thu Jun 16 17:33:47 2022 +0100

    Update theme.less

commit 0996e75
Author: Jan Faracik <[email protected]>
Date:   Thu Jun 16 17:32:52 2022 +0100

    Update theme.less

commit e57bc28
Author: Jan Faracik <[email protected]>
Date:   Thu Jun 16 17:29:15 2022 +0100

    Update theme.less

commit 69c1633
Author: Jan Faracik <[email protected]>
Date:   Thu Jun 16 17:00:06 2022 +0100

    Update style.less

commit e21d4e7
Author: Jan Faracik <[email protected]>
Date:   Wed Jun 15 19:03:50 2022 +0100

    Update breadcrumbs to use new item mixin

commit 90fc4ba
Author: Jan Faracik <[email protected]>
Date:   Wed Jun 15 18:57:51 2022 +0100

    Rename mixin

commit 28d0638
Merge: d1f42ac 65fcda1
Author: Jan Faracik <[email protected]>
Date:   Tue Jun 14 23:35:10 2022 +0100

    Merge branch 'master' into move-manage-jenkins-less

commit d1f42ac
Author: Jan Faracik <[email protected]>
Date:   Fri Jun 10 11:13:58 2022 +0100

    Update table.less

commit 13d5729
Author: Jan Faracik <[email protected]>
Date:   Thu Jun 9 10:03:01 2022 +0100

    Move from px to rem

commit 68f5425
Author: Jan Faracik <[email protected]>
Date:   Wed Jun 8 16:40:41 2022 +0100

    Update theme.less

commit 929e2d5
Author: Jan Faracik <[email protected]>
Date:   Wed Jun 8 16:28:06 2022 +0100

    Update mixins.less

commit 8c4b466
Merge: 8b22270 77a36fc
Author: Jan Faracik <[email protected]>
Date:   Wed Jun 8 16:27:25 2022 +0100

    Merge branch 'master' into move-manage-jenkins-less

commit 8b22270
Author: Jan Faracik <[email protected]>
Date:   Wed Jun 8 16:19:42 2022 +0100

    Update theme.less

commit e925490
Author: Jan Faracik <[email protected]>
Date:   Wed Jun 8 16:17:02 2022 +0100

    Add mixin for items

commit f89fe63
Merge: 1cb37d1 032a105
Author: Jan Faracik <[email protected]>
Date:   Thu May 26 20:18:41 2022 +0100

    Merge branch 'master' into new-plugin-manager-1

commit d579dc5
Author: Jan Faracik <[email protected]>
Date:   Tue May 17 00:52:29 2022 +0100

    Update section.less

commit 43e4f5c
Author: Jan Faracik <[email protected]>
Date:   Tue May 17 00:29:22 2022 +0100

    Init

commit 1cb37d1
Author: Jan Faracik <[email protected]>
Date:   Thu Apr 28 17:23:38 2022 +0100

    Revert "Remove unused icon preload"

    This reverts commit 28aaf91.

commit 28aaf91
Author: Jan Faracik <[email protected]>
Date:   Thu Apr 28 17:20:41 2022 +0100

    Remove unused icon preload

commit dc8ba52
Author: Jan Faracik <[email protected]>
Date:   Thu Apr 28 12:07:35 2022 +0100

    Fix translucency on Firefox

commit 84596ee
Author: Jan Faracik <[email protected]>
Date:   Mon Apr 25 20:37:20 2022 +0100

    Update plugin-manager.less

commit cf28fee
Author: Jan Faracik <[email protected]>
Date:   Thu Apr 21 23:33:22 2022 +0100

    Add loading animation

commit addb738
Author: Jan Faracik <[email protected]>
Date:   Thu Apr 21 23:23:13 2022 +0100

    Finish

commit 53902db
Author: Jan Faracik <[email protected]>
Date:   Thu Apr 21 23:02:08 2022 +0100

    Update sidebar

commit b8a622c
Author: Jan Faracik <[email protected]>
Date:   Thu Apr 21 22:43:32 2022 +0100

    Init
@NotMyFault NotMyFault added the rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted label Sep 6, 2022
@timja
Copy link
Member

timja commented Sep 6, 2022

@NotMyFault can you label these with web-ui when you do it please?

Useful for filtering these PRs 😄

@timja timja added the web-ui The PR includes WebUI changes which may need special expertise label Sep 6, 2022

const pageSearchBar = document.querySelectorAll(".jenkins-search__input");
if (pageSearchBar.length === 1) {
hotkeys(translateModifierKeysForUsersPlatform("/"), () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I press my key combination to generate a slash, I have no selection or focus. Am I missing something?

I used the following snipped to check whether my input is considered:

--- a/war/src/main/js/keyboard-shortcuts.js
+++ b/war/src/main/js/keyboard-shortcuts.js
@@ -15,7 +15,9 @@ window.addEventListener("load", () => {
 
   const pageSearchBar = document.querySelectorAll(".jenkins-search__input");
   if (pageSearchBar.length === 1) {
-    hotkeys(translateModifierKeysForUsersPlatform("/"), () => {
+    let input = "t"
+    hotkeys(translateModifierKeysForUsersPlatform(input), () => {
+      console.log(input)
       pageSearchBar[0].focus();

My input with "/" wasn't considered at all, I had to change it to something else. I'm using a letter as a working example.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In terms of ff4d685, what kind of keyboard do you use, where CMD is used for a slash @timja 👀
Isn't that typically done via Shift (and equally shift+option for a backslash), at least that's the case on my macbook.

Copy link
Contributor Author

@janfaracik janfaracik Sep 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated this to include a singular slash without CMD @timja - is that all good with you? Trying to be consistent with other web services which use / to search.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm the singular / didn’t work for me in chrome or safari on Mac

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok it's working for me now, I assume it was cached locally. I've reverted the commits

@timja timja requested review from NotMyFault and a team September 6, 2022 21:24
@NotMyFault NotMyFault added the needs-security-review Awaiting review by a security team member label Sep 6, 2022
@NotMyFault NotMyFault requested a review from a team September 6, 2022 21:56
@daniel-beck
Copy link
Member

Looks good 👍

Unsure about the shortcut and related API design. Especially if the build history filter is one of the fields for this that has a shortcut, the chance will be pretty big that two unrelated fields are on the same page. Wouldn't they benefit from customizable shortcuts, or a way to specify their ordinal/priority in getting the shortcut? Unless shortcuts are fully configurable, they could be going to a different input than expected, instead of no input, is also weird. Unsure which is better, but mentioning it for consideration.

Is it deliberate to have so completely different designs involving similar functionality in similar fields between the Quicksilver search on the top right, and the new filter fields (rectangle with a character inside on the right vs. (Modifier + K) just after placeholder.

@timja
Copy link
Member

timja commented Sep 7, 2022

they could be going to a different input than expected, instead of no input, is also weird

the current implementation disables the shortcut if there's more than one search-bar on the page

@janfaracik
Copy link
Contributor Author

Is it deliberate to have so completely different designs involving similar functionality in similar fields between the Quicksilver search on the top right, and the new filter fields (rectangle with a character inside on the right vs. (Modifier + K) just after placeholder.

Wanting to overhaul the search functionality (hopefully soon™) so I've left the current search bar as-is. The replacement uses this new component and looks somewhat similar to the below (the below is almost a year old though):

Screenshot 2022-09-07 at 09 58 44

https://www.youtube.com/watch?v=SxSObgwjwt8&t=3808s

@daniel-beck
Copy link
Member

they could be going to a different input than expected, instead of no input, is also weird

the current implementation disables the shortcut if there's more than one search-bar on the page

Right, that was referring to my suggestion to have ordinal/priority between fields in getting the shortcut assigned to them.

timja added a commit to jenkinsci/acceptance-test-harness that referenced this pull request Sep 8, 2022
Copy link
Member

@timja timja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great ❤️

The keyboard shortcut seems to just be a caching issue, when you swap between branches you need to force refresh assets.

ATH testing passed (jenkinsci/acceptance-test-harness#914)

Copy link
Member

@NotMyFault NotMyFault left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Marvelous work 🎉

@timja
Copy link
Member

timja commented Sep 13, 2022

@jenkinsci/core-security-review could you check this please


const pageSearchBar = document.querySelectorAll(".jenkins-search__input");
if (pageSearchBar.length === 1) {
hotkeys(translateModifierKeysForUsersPlatform("/"), () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary call to translateModifierKeysForUsersPlatform?

@daniel-beck daniel-beck added security-approved @jenkinsci/core-security-review reviewed this PR for security issues and removed needs-security-review Awaiting review by a security team member labels Sep 14, 2022
@github-actions github-actions bot added the unresolved-merge-conflict There is a merge conflict with the target branch. label Sep 14, 2022
@github-actions
Copy link
Contributor

Please take a moment and address the merge conflicts of your pull request. Thanks!

@github-actions github-actions bot removed the unresolved-merge-conflict There is a merge conflict with the target branch. label Sep 14, 2022
@timja
Copy link
Member

timja commented Sep 14, 2022

/label ready-for-merge


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

Thanks!

@comment-ops-bot comment-ops-bot bot added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Sep 14, 2022
@timja timja merged commit 8ab0519 into jenkinsci:master Sep 16, 2022
@timja timja deleted the add-search-bar-component branch September 16, 2022 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted security-approved @jenkinsci/core-security-review reviewed this PR for security issues web-ui The PR includes WebUI changes which may need special expertise
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants