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

[fix] keyboard navigation / simple theme (UI) Fix 2751 & 2788 #2790

Merged
merged 2 commits into from
Sep 16, 2023

Conversation

BernieHuang2008
Copy link
Contributor

What does this PR do?

fixed both #2751 and #2788 at the same time .

By finding the next result manually insdead of using .nextElementSibling.

Why is this change important?

fixed bugs and improve UX

How to test this PR locally?

search the keyword below and try to navigate between groups.

Author's checklist

Related issues

Closes #2751
Closes #2788

@return42
Copy link
Member

Wow, very cool .. I squashed your third commit into you first commit and modified the commit message .. hope thats OK for you ..

One point I have, I think the previous-/next-ElementSibling is superfluous here and the if-condition can be remove .. what to you think about this patch of your code? -->

diff --git a/searx/static/themes/simple/src/js/main/keyboard.js b/searx/static/themes/simple/src/js/main/keyboard.js
index ae324aadd..5c02acbfd 100644
--- a/searx/static/themes/simple/src/js/main/keyboard.js
+++ b/searx/static/themes/simple/src/js/main/keyboard.js
@@ -234,16 +234,10 @@ searxng.ready(function () {
           }
           break;
         case 'down':
-          next = current.nextElementSibling;
-          if (next === null) {
-            next = results[results.indexOf(current) + 1];
-          }
+          next = results[results.indexOf(current) + 1] || current;
           break;
         case 'up':
-          next = current.previousElementSibling;
-          if (next === null) {
-            next = results[results.indexOf(current) - 1];
-          }
+          next = results[results.indexOf(current) - 1] || current;
           break;
         case 'bottom':
           next = results[results.length - 1];

@BernieHuang2008
Copy link
Contributor Author

can't agree with you more ~

BernieHuang2008 and others added 2 commits September 16, 2023 13:08
- avoid loop select
- fix select next item in mixed result lists

Replaces: searxng#2789
Closes: searxng#2751
Closes: searxng#2788
@return42
Copy link
Member

can't agree with you more ~

:) .. I squashed my patch from above into your commit (see diff) and rebuild the static files ..

Before we merge .. can you please check if my modification do work as expected (in all edge cases) / thanks!

@return42 return42 changed the title Fix 2751 [fix] keyboard navigation / simple theme (UI) Fix 2751 & 2788 Sep 16, 2023
@BernieHuang2008
Copy link
Contributor Author

tested, everything right.

@return42
Copy link
Member

tested, everything right.

OK / thanks! .. I'm going to merge .. I want to have this merged, before we going to start the preferences --> #2788 (comment)

@return42 return42 merged commit 89fbac5 into searxng:master Sep 16, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Problems about Keyboard Navigation Arrow-Keys (and vim hotkeys) do not work in mixed result lists
2 participants