[feature] ArrowKeys to navigate between image results #2724
Conversation
BernieHuang2008
changed the title
|
Local check passed, require check approvals. |
|
good job. i tried the code and it seems to work well. IMO what you mentioned in the video is not an important detail, you shouldn't stress it. i think the code is ready. |
|
Thank you, i think it's ready too. |
BernieHuang2008
changed the title
BernieHuang2008
changed the title
BernieHuang2008
changed the title
|
@return42 @dalf @bonswouar @Hackurei |
|
Looks good to me! As you said the easy fix would be to bind on document, but I feel like this is not very clean (we would have the event catch everywhere on the site every time a key is pressed, with a check for existing div). And I don't really know why binding on If someone with more knowledge about js good practises can share it's thought he's more than welcome :) |
|
With all your tests and comments, I've been able to fix the issue when the user clicks on empty space like described in #2736 : |
That's cool and beautiful, I like it. |
why not make the changes here? |
|
i've updated @dalf 's changes, to this PR. |
|
@BernieHuang2008 can you squash your commits and call |
BernieHuang2008
force-pushed
the
master
branch
from
c97e755
to
8051277
Compare
|
@dalf i've squashed and built. |
|
@BernieHuang2008 can you possibly squash to 1 commit? |
BernieHuang2008
force-pushed
the
master
branch
from
317fc60
to
fd48814
Compare
|
ok, squashed into 1 commit, you can now merge the PR if nothing wrong. |
BernieHuang2008
force-pushed
the
master
branch
2 times, most recently
from
4473c77
to
9d8c1d1
Compare
Hy @BernieHuang2008, thanks a lot for your work on this topic 👍 .. and sorry for my absent the last days ..
Don't worry about .. I will do it in my review I started right now. |
|
FYI I removed the build files from your commit and rebased your commit on SearXNG's master branch / I'm still in review .. to be continued |
- KeyboardEvent: keyCode property is depricated, replaced by key property [2] - the check for ifDetailOpened is not necessary, because the hotkeys are not only applicable to image-results, by example: `!goi !go !scc hello` - Key bindings like h for help are to be used in general (not only in vim-mode) [1] https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/keyCode [2] https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/key Signed-off-by: Markus Heiser <markus.heiser@darmarit.de>
the hotkeys should not only applicable to image-results / I pushed 3eff63f on top of your branch .. read commit message for more details. The hotkeys now are ...
Related: But there is more work to do, there is an old issue: I will fix it in a follow up PR. |
|
@BernieHuang2008 @bonswouar can you please test my modifications .. if you think everything is OK, we should merge this PR. |
Sure,please wait a moment ... |
|
Something wrong with my codespaces, please wait |
May be you have to checkout your master branch again since I forced pushed to your master branch when I removed the build files from your commit. |
|
Thank you I've found it. I think it works well. But i still have the question, mentioned in the review --- using |
I wrote it down in the commit message: 3eff63f |
sorry i missed it. I agree with you now. |
|
Looks good! |
I think we sometimes need this, especially in the "general" mode. |
Personally I don't mind no animation! |
|
Thanks to all who had worked on this topic 👍 .. and made SearXNG better and better :) I'm going to merge the PR now .. |
What does this PR do?
Improved PR #2721.
Why is this change important?
How to test this PR locally?
Author's checklist
I use Switch-case to make the code more readable and flexible, it's completely same as the example @return42 gived in #2721.
Related issues
Closes #2720
Closes #2736