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(search-box): make search results scrollable #11233

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

pseudopilot
Copy link
Contributor

@pseudopilot pseudopilot commented Jun 2, 2024

Summary

Fixes #5650
(the second (remaining) part of it)

Problem

When search results overflow the viewport the navigation by arrow UP and DOWN buttons across the list doesn't scroll the page. As a result we can't see (without additional page scrolling) which search result option outside viewport is currently selected.

Solution

The outer container of search results is calculated the way that it never overflows the viewport and become scrollable if needed. It allows to scroll the list of results via arrow UP and DOWN buttons keeping selected option always in the viewport.

Before

Screen.Recording.2024-06-02.at.16.11.06.mov

After

Desktop:

Screen.Recording.2024-06-02.at.16.14.21.mov

Mobile:

Screen.Recording.2024-06-02.at.16.16.40.mov

@pseudopilot pseudopilot force-pushed the make-search-results-scrollable branch 3 times, most recently from afc33c0 to b2fd8f1 Compare June 2, 2024 15:54
@pseudopilot pseudopilot force-pushed the make-search-results-scrollable branch from b2fd8f1 to ce00d59 Compare June 2, 2024 15:58
@pseudopilot pseudopilot marked this pull request as ready for review June 2, 2024 16:14
@pseudopilot pseudopilot requested a review from a team as a code owner June 2, 2024 16:14
@caugner caugner added ux make the user experience awesome involves: UX Requires the attention of the UX team. 🔎 search Search feature labels Jun 4, 2024
@fiji-flo
Copy link
Contributor

fiji-flo commented Jun 6, 2024

Hey, nice work. I see the pain point. I think this is actually a CSS issue.

If you just use:

.header-search .search-results {
    max-height: calc(100vh - var(--sticky-header-without-actions-height) - 4rem);
    overflow-y: auto;
}
``

the issues should be fixed without any JS. Could you look into that?

@pseudopilot
Copy link
Contributor Author

pseudopilot commented Jun 10, 2024

max-height: calc(100vh - var(--sticky-header-without-actions-height) - 4rem);

@fiji-flo , just checked. It does fix the problem for the header search, but doesn't fix for the home page search:

Screen.Recording.2024-06-10.at.23.46.44.mov

I'm sure (from my own experience) that the header search is being used much more often. So, do we want to fix this only for it just with CSS? If yes, I'll update my PR.

@github-actions github-actions bot added the idle label Jul 11, 2024
@github-actions github-actions bot removed the idle label Aug 16, 2024
@github-actions github-actions bot added the idle label Sep 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
idle involves: UX Requires the attention of the UX team. 🔎 search Search feature ux make the user experience awesome
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Search box is not keyboard friendly
3 participants