Bring search bar into view on desktop #1872
Conversation
| @@ -161,10 +161,21 @@ export function getProjectNameAndVersion () { | |||
| } | |||
|
|
|||
| /** | |||
| * Return `true` if the client's OS is MacOS | |||
| * Return `true` if the client's OS is MacOS. | |||
There was a problem hiding this comment.
Just added a missing period from my last PR :)
|
I noticed a problem with this PR I thought I had taken care of so I've converted it to a draft until I fix it. It looks like I may need to bring in click-away since click the clear button or gear causes the whole search bar to close. |
Could we just naturally make it so scrolling down makes it disappear again? If we scroll up, it continues at the top, if we scroll down, it goes away. The idea is that it behaves like in mobile, except that scrolling up on large screens does not bring it down, only pressing "/". |
|
And thank you on the PR and the work so far! |
Absolutely! It certainly crossed my mind in the several ways I realized this could work so I'll go with that as it will also solve my other problem. |
|
Just an update: I'm very sorry I have not gotten back to this. I will in the next day or so! I just had some stuff happen that is making me not want to write code and be social, even though for today that has manifested in talking about code a whole lot of Elixir Forum 🙃 |
sodapopcan
force-pushed
the
search-bar-ux
branch
from
2f6464d to
67fe224
Compare
| padding: 16px 12px 18px 19px; | ||
| } | ||
|
|
||
| body.sidebar-closed .sidebar-button.fixed-top { |
There was a problem hiding this comment.
The burger menu has to be moved downward to be properly aligned with the searchbox when we are scrolled down the page. When we are at the top of the page, the top-search div is padded differently so we have to compensate in this situation.
This commit changes how the search bar is brought into view on desktop as brought up in elixir-lang#1860: when using one of the keyboard shortcuts to focus the input, the entire page jumps to the top making you lose your scroll position. On mobile, technically smaller screens, this is remedied by causing the input to slide into view on scroll up, but I don't believe this is a desirable solution for desktop. The following changes are introduced: * Using one of the keyboard shortcuts will focus the search input causing it to stick to the top. * It will continue to stick to the top so long as it is focused allowing you to scroll with it open. * The slide-on-scroll effect has been changed to only fire on touch-enabled devices as opposed to just smaller screens. This is allows us to make the hexdocs window very small and still use keyboard shortcuts--Useful on laptops. Closes elixir-lang#1860
sodapopcan
force-pushed
the
search-bar-ux
branch
from
67fe224 to
930def0
Compare
|
|
||
| if (!isTouchDevice()) { | ||
| qs(TOP_SEARCH_SELECTOR).classList.add('sticky') | ||
| if (window.scrollY === 0) { |
There was a problem hiding this comment.
See comment above.
| backgroundLayer.classList.remove('sm-hidden') | ||
| } | ||
| } else if (currentScroll !== lastScrollTop) { | ||
| topSearch.classList.remove('sticky') |
There was a problem hiding this comment.
I was going to extract these into a closeSearchbar function but it's only called once and it would require passing in the DOM elements as dependencies otherwise we'd running three DOM queries in a scroll callback which would be worse than this run-on sentence :)
|
This is ready for review! This was honestly more of a slog than I imagined it to be 😰 Certainly not the worst, but there are more states to consider than I was thinking. I only mention that as I'm a bit burned out on it so I never brought in the animation. That said, if for some reason you want to reject this don't worry, I won't take it poorly (I know you're probably used to this I always just like to reassure for open source contributions). I added some review comments and otherwise I think it's best to just try it out as opposed to me trying to describe it, but here is a summary anyway: Desktop
Mobile
As per that last point, all behaviour related to showing the searchbar is dependent on being on a touch-enabled device or not and nothing to do with viewport size. |
|
💚 💙 💜 💛 ❤️ |
This commit changes how the search bar is brought into view on desktop as brought up in #1860: when using one of the keyboard shortcuts to focus the input, the entire page jumps to the top making you lose your scroll position. On mobile, technically smaller screens, this is remedied by causing the input to slide into view on scroll up, but I don't believe this is a desirable solution for desktop.
The following changes are introduced:
PR notes
This turned out to be a little hairier than I imagined due to thinking there was more that needed to be done than there was. Turns out it's a fairly small PR! I did have to make the following concessions but overall I feel the UX is pretty solid.