Stick kiwixNav on top #896
Conversation
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## main #896 +/- ##
=======================================
Coverage 72.00% 72.00%
=======================================
Files 54 54
Lines 3751 3751
Branches 2096 2096
=======================================
Hits 2701 2701
Misses 1048 1048
Partials 2 2 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
There was a problem hiding this comment.
On narrow screens the topbar section's use of the screen space is quite uneconomic:
Making the topbar sticky worsens the situation.
I propose that on narrow screens
- the topbar's vertical space usage is fixed and
- the topbar is autohidden while scrolling (similar to how ZIM viewer's toolbar behaves).
|
@juuz0 Any feedback on @veloman-yunkan request? This PR is part of the next milestone and have only a few tickets left. Therefore we should move forward quickly on this. If you could rebase, that woukd be great as well. |
|
Sorry I totally missed this review comment while working on other PR, doing it ASAP |
|
@juuz0 Thx! Github provides great notification features. I recommend to any Kiwix dev to put attention on this. If you don't do this, then this just slows down artificially the whole development process and create additional work load to others (like you know, it is not really the role of the CTO or any other dev to chase a colleague so he communicates transparently about his own work). |
Understandable. Apologies. |
|
Edit: done. |
I put them inside |
static/skin/index.js
Outdated
| if (st > previousScrollTop) { | ||
| kiwixNav.style.position = 'fixed'; | ||
| kiwixNav.style.top = '-100%'; | ||
| document.querySelector('.kiwixHomeBody').style.width = '100%'; |
There was a problem hiding this comment.
Why do you need this line here? Is the width of kiwixHomeBody element set dynamically to something else somewhere else?
There was a problem hiding this comment.
I don't think (and have checked) it's dynamically set anywhere. But still, when I remove kiwixNav all the list, gets shifted to the left on scrolling. Checking through inspector, the width is reduced hence the line.
There was a problem hiding this comment.
Can't it be set in the CSS instead?
There was a problem hiding this comment.
You're right. It can be done in CSS & should've been done there only. Thank you.
At such moments, I realise I shouldn't haste.
The filters menu will always stay on top now.
No pre defined height for devices with with max-width 590px now. The previous height took a good amount of space on some devices.
juuz0
force-pushed
the
stickyNav
branch
2 times, most recently
from
408f089
to
8ed16ed
Compare
Since kiwixNav is sticky for larger screens now, the tiles area on mobile devices is incredibly low. This change hides kiwixNav if the screen is scrolled.
Fixes #883