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

Remove shortcut indicator search dialog on small screens #1947

Merged
merged 2 commits into from
Aug 15, 2024

Conversation

timhoffm
Copy link
Contributor

@timhoffm timhoffm commented Aug 6, 2024

Closes #1944.

Alternative to #1945. While #1945 removes the shortcut completely, I've suggested that it actually has value and should only be removed on smaller screens. That's what this PR implements. It shows the shortcuts if the window size is at least "md" i.e. >=768px (I figured, this is a reasonable threshold - see discussion below, but if you think other values are better, that's perfectly fine).

Search on small screens (IPhone Pro Max size)
image

The minimum width with shortcuts is illustrated in the following screenshot. With this the search field is large enough that the shortcut on the right is not cluttering/distracting from the "Search the docs ..." message. On the other end, I'd consider that size about the minimum size a user would give a browser window to read docs. If the window size is smaller, it's unlikely that the user is using a "large" device with a keyboard attached, and hence the shortcut is not needed.
image

@timhoffm timhoffm marked this pull request as draft August 6, 2024 15:42
@timhoffm timhoffm marked this pull request as ready for review August 6, 2024 16:02
@timhoffm
Copy link
Contributor Author

timhoffm commented Aug 8, 2024

Is there anything to do on my side? The CI failure seems unrelated.

@Carreau
Copy link
Collaborator

Carreau commented Aug 12, 2024

Failure will be fixed by spatialaudio/nbsphinx#807, so nothing to do on your side I think.

Copy link

Coverage report

This PR does not seem to contain any modification to coverable code.

@Carreau
Copy link
Collaborator

Carreau commented Aug 15, 2024

Reran failed test, it's passing now. Merging.

@Carreau Carreau merged commit 019ad51 into pydata:main Aug 15, 2024
29 checks passed
@timhoffm timhoffm deleted the patch-2 branch August 15, 2024 13:27
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.

Remove shortcut hint in search field on mobile
3 participants