django
django copied to clipboard
Fixed #32934 -- Set admin sidebar height to be dynamic based on visible header
A pure CSS solution for ticket #32934 addressed in PR #14281 instead of the javascript approach in the previous PR.
This solution uses calc()
to responsively set the height of the navigation sidebar to be 100vh
minus whatever the height of the header is, so that it does not extend past the fold, making scrolling through it a little easier. It uses a media query at 1024px.
One edge case is when the title of the header has been replaced with some very long text, which increases the height of the header even further. In that case, we just revert back to the current behavior and the navbar extends past the fold. This edge case is expected to be very rare, since the amount of text required in the header is long.
This achieves the previous PR's expected result - Before:
After:
Hello @hrushikeshrv! Thank you for your contribution 💪
As it's your first contribution be sure to check out the patch review checklist.
If you're fixing a ticket from Trac make sure to set the "Has patch" flag and include a link to this PR in the ticket!
If you have any design or process questions then you can ask in the Django forum.
Welcome aboard ⛵️!
@hrushikeshrv I can see a strange side-effect of this patch, the sidebar is cut off at the bottom on the page: Django 4.1b1:
@felixxm yes, reducing the height of the navbar leads to it being cut-off when the user scrolls down (since the height is no longer 100vh
). Unfortunately, I think this is a limitation of CSS and/or sticky positioning, and I don't think there is any pure CSS fix for this issue (not 100% sure about that, though, but around 99% sure). One workaround might be also making the header and the breadcrumbs sticky, for e.g. -
In my opinion, this "feels" better to scroll through, since the header only takes around 120px
at most, and it also keeps the "VIEW SITE", "CHANGE PASSWORD", "LOGOUT" links, as well as the breadcrumbs visible even on scroll, and also doesn't cause the navbar to look cut-off when the user scrolls. We could do this with some media queries, but I think a better solution could also involve refactoring app_list.html
and/or app_index.html
a little.
Hi @hrushikeshrv — thanks for the effort here. Sorry for the pause in follow-up our end: it's one of those tricky ones.
Looking at this again, I think the issue that @felixxm points out is telling: once the header is scrolled away, the gap at the bottom of the sidebar isn't good at all.
Thus, I think we have to say that this simple approach isn't acceptable.
...and I don't think there is any pure CSS fix for this issue
We could do this with some media queries, but I think a better solution could also involve refactoring app_list.html and/or app_index.html a little.
In that case I think perhaps this isn't worth fixing.
On main
, as it currently stands, if I scroll the left-sidebar it pauses, and begins to scroll in sync with the main content, until the bottom is reached... — yes, OK, that's not 100% independent scrolling... but I think it's OK. That's how just browsers work, I might think. (I don't see it much different from https://github.com/django/django/pull/14281#issuecomment-947378429)
So we have a minor issue, which looks like a full-workaround requires a quite large fix. I'm just not seeing that as warranted.
So I think we should mark the ticket as wontfix
, pending as actual proposal for a simple fix that covers the issues mentioned here and on #14281. If you still fancy working on it, that's super: if you hit a good solution we can easily reopen.
Thanks again.
I agree with the wontfix
for this. The work involved for fixing it would be a lot for a minor issue. I'll keep it in the back of my mind in the meanwhile in case I think of something!
Great. Thanks @hrushikeshrv 👍