Navigate search modal using up and down keys #2761
Conversation
|
The UI looks pretty good, should we add a border while keyboard navigation? CC @mortenpi |
There was a problem hiding this comment.
Can't we just use, element.focus()?
Would eliminate all this extra logic from both js and scss...?
https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/focus
There was a problem hiding this comment.
If I am using focus, I am getting this blue white border like :
which I am not being able to remove I have done border: none, outline:none still it is there
There was a problem hiding this comment.
most probably because these are < a > tags so there will always be an unavoidable outline
There was a problem hiding this comment.
You can remove the border using the <element_classname>:focus {border:none;}.
Also, I like the idea of the border it's not subtle(maybe we can make it) since it kinda highlights the current selection more than just the hover styles.
There was a problem hiding this comment.
oh it works now, I forgot to run make themes so the changes were not reflecting, now the outline's gone
There was a problem hiding this comment.
there's a subtle border idea or one more thing I was thinking of was we can also create a shadow effect for the selected result. What do you think? I am fine with both or neither whatever you recommend
There was a problem hiding this comment.
Ia it possible to get a small demo recording for both options, subtle border and shadow?
There was a problem hiding this comment.
This is with the border
Screen.Recording.2025-08-20.at.12.47.11.PM.mov
This is with the shadow
Screen.Recording.2025-08-20.at.12.50.42.PM.mov
There was a problem hiding this comment.
I kinda like the border FWIW, but @Hetarth02 feel free to make the call here.
There was a problem hiding this comment.
We went ahead with the border one.
|
This looks really nice! @Hetarth02, if you're happy with the UI, we can merge whenever as far as I am concerned. |
| .search-result-link:hover, .search-result-link:focus { | ||
| background-color: $search-result-link-hover; | ||
| outline : none; | ||
| border: 2px solid #1DD2AF; |
There was a problem hiding this comment.
Can we use a variable instead of hard-coded values for colors?
Also, would be good to add relevant color values for different themes.(but not super important)
There was a problem hiding this comment.
replaced the hard-coded value with a variable in _variables.scss
There was a problem hiding this comment.
also checked every theme, the colors are looking good
There was a problem hiding this comment.
We went ahead with the border one.
|
Thank you for the commit @mortenpi 😁 |
|
The nightly failures look unrelated. But the search benchmark one is also still failing. |
|
PrettyTables.jl released a new version 4 days ago, that caused the ci issue :) |
|
Thanks a lot for this @Rahban1 and @Hetarth02. Sorry for the delay in merging. |
|
Thank you @mortenpi 👍 |
3 participants
as discussed in the biweekly meeting @mortenpi @Hetarth02
this is how it looks like locally for me :
Screen.Recording.2025-08-19.at.4.44.14.PM.mov