Opened 5 years ago

Closed 4 years ago

Last modified 11 days ago

#32934 closed Bug (wontfix)

Fix Admin's sidebar height to be dynamic based on visible header

Reported by: gary Owned by: Hrushikesh Vaidya
Component: contrib.admin Version: 3.2
Severity: Normal Keywords:
Cc: Naresh Kumar Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: yes UI/UX: yes

Description

The current sticky nav bar on admin anchors past the page. If you have long list of apps you have to scroll the whole page and then the nav to get to the bottom.

Images, context, and patch available at ​https://github.com/django/django/pull/14281

Change History (8)

comment:1 by Jacob Walls, 5 years ago

Owner: changed from nobody to gary
Status: new → assigned
Triage Stage: Unreviewed → Accepted

Thanks -- accepting based on Tom's review of the ​PR.

comment:2 by Carlton Gibson, 5 years ago

Patch needs improvement: set

Current approach on PR is overly complex. A simpler CSS-only approach should be feasible using a media query to handle different display sizes.

Last edited 5 years ago by Carlton Gibson (previous) (diff)

comment:3 by Carlton Gibson, 5 years ago

Easy pickings: set
Owner: gary removed
Status: assigned → new

comment:4 by Hrushikesh Vaidya, 5 years ago

Owner: set to Hrushikesh Vaidya
Status: new → assigned

A CSS only approach ​PR

comment:5 by Mariusz Felisiak, 4 years ago

Patch needs improvement: unset

comment:6 by Mariusz Felisiak, 4 years ago

Patch needs improvement: set

comment:7 by Carlton Gibson, 4 years ago

Resolution: → wontfix
Status: assigned → closed
Triage Stage: Accepted → Unreviewed

I'm going to resolve this as wontfix.

The scrolling issue doesn't seem that bad: the left sidebar pauses independent scrolling at a point, scrolling in sync with the main section until reaching the bottom (of either) after that, so it's not 100% independent scrolling, but its OK. Perhaps it's Just what browsers do, and given #32204 we have the filter in-place now too, it's a minor glitch at worst.

In contrast the fixes are either too heavy (​PR14281) or else have other display issues (​PR15234) and so it's not clear there's a simple fix available (one that's really justified by the issue). Pending such actually showing up, I don't further effort here is the best use of time.

Happy to reopen to consider patches.

Version 1, edited 4 years ago by Carlton Gibson (previous) (next) (diff)

comment:8 by Naresh Kumar, 11 days ago

Cc: Naresh Kumar added

Hi Carlton, I ran into this as well and would like to take another look.

The earlier attempts ran into the problem that the header height isn't fixed
(breakpoints, wrapped breadcrumbs), so a static CSS value can't work.

I have a small patch: about 25 lines in nav_sidebar.js and no CSS changes.
It sets the sidebar's max-height from its actual offset from the top of the
viewport, updated on scroll/resize via requestAnimationFrame, so it adapts
to any header height both at scroll position 0 and when scrolled. It comes
with a Playwright regression test that fails without the patch and passes
with it (checked on Chromium, Firefox and WebKit).

Would you be open to reopening the ticket for this approach, or do you feel
any JS here is more than the issue warrants? I won't open a PR unless you
think it's worth reviewing.

Note: See TracTickets for help on using tickets.
Back to Top