Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Make sidebar scrollable and sticky (on modern browsers)#91

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
JulienPalard merged 3 commits intopython:masterfromdlukes:fix-sidebar
Jan 11, 2022

Conversation

dlukes
Copy link
Contributor

Fixes#20.

Just make the sidebar stick to the top with CSS and show a scrollbar onoverflow. There's no fallback for older browsers -- since up to now,this didn't work for anyone (because "intelligent" scrolling has beenbroken for years), I assume improving the UX at least for modernbrowsers is still a net improvement.
@the-knights-who-say-ni

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed thePSF contributor agreement (CLA).

CLA Missing

Our records indicate the following people have not signed the CLA:

@dlukes

For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please followthe steps outlined in the CPython devguide to rectify this issue.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

You cancheck yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

@dlukes
Copy link
ContributorAuthor

For more info, seemy comment on the associated issue.

@JulienPalard
Copy link
Member

(The buidl artifact is available here for local tests :https://github.com/python/python-docs-theme/actions/runs/1434325782)

@JulienPalard
Copy link
Member

I have a sidebar separator issue when scrolling to the bottom, see:

Screenshot 2021-11-25 at 17-54-46 3 9 9 Documentation

(Maybe this separator can just be removed now that the theme is responsive, but I think it's worth a different PR?)

@dlukes
Copy link
ContributorAuthor

Hmm, interesting. What OS/browser? I'm asking because I can't reproduce this on Linux on either Firefox or Chrome:

image

The sidebar still overflows a bit as a result of the switch tobox-sizing: border-box (I think), which is definitely also worth a fix now that I see it, but something tells me it won't help with the much more dramatic overflow you seem to be experiencing.

Just to make sure: if you accesshttps://docs.python.org, I suppose the sidebar renders correctly?

(Maybe this separator can just be removed now that the theme is responsive, but I think it's worth a different PR?)

Definitely worth a separate PR, if at all :) Personally, I don't think I've ever clicked it, but I've seen comments in threads concerning the Python docs theme from people who seem very fond of it, and removing it would undoubtedlybreak their workflow.

@dlukes
Copy link
ContributorAuthor

dlukes commentedNov 25, 2021
edited
Loading

OK, so right after I posted my previous reply, I accidentally stumbled upon a way to trigger the behavior you're experiencing... Sigh, typical.

It's sort of weird though -- for some reason, the sidebar gets out of whack whenever a browser UI element pops up at the bottom (???), e.g. the search bar:

image

Or the dev tools pane:

image

I've no idea how or why this is happening but I'll investigate.

(I'm guessing something funky is happening with the sidebar's JavaScript, which makes me want to get rid of the whole thing, but see above.)

@dlukes
Copy link
ContributorAuthor

Oh, and zooming in on the page also triggers the behavior you've described.

@hugovk
Copy link
Member

I can reproduce as well on macOS Monterey with Firefox, Chrome and Safari.

To repro I needed to reduce the height of the window:

image

Or open a page with content longer than the window height:

image

@dlukes
Copy link
ContributorAuthor

Sorry for the long turnaround, it's a busy time of the year, my wife and I are expecting a second daughter in a month or so, etc. :) Please take a look at the fix I just pushed, when you get a chance, and let me know if it works for you,@JulienPalard and@hugovk.

It should even fix an existing bug in the placement of the «, which is meant to stay put vertically centered in the viewport, but gets out of whack when you adjust the zoom level of the page.

Other than that, should I worry about what happens when using a browser that doesn't support viewport units? Especially w.r.t. to the collapse button -- honestly, I'd rather not put too much work into making it behave nicely on older browsers if it's going to be axed at some point in the future anyway.

@JulienPalard
Copy link
Member

(a build artefact is available for local tests in the "Checks" tab)

dlukes reacted with thumbs up emoji

@hugovk
Copy link
Member

Looks good now, thanks!

image

image

@dlukes
Copy link
ContributorAuthor

dlukes commentedDec 13, 2021
edited
Loading

Thanks! I've noticed another issue though -- when the content is short, the sidebar still stretches to full viewport height. I mean, that's what the CSS says, so of course it does :) Here's what it looks like, my patch on the left, current version on the right:

image

I assume we'd rather keep the current behavior? Adding so much empty space is not exactly broken, but it's definitely unnecessary.

@dlukes
Copy link
ContributorAuthor

I've pushed a fix which relies on flexbox instead of floats for the sidebar layout. Let me know what you think.

@JulienPalard
Copy link
Member

LGTM, tested on Firefox 95 (on Debian (desktop)) and on Firefox 91 (on Mobian (phone)).

I'd appreciate if someone else could take a look at it before merging, I don't trust my skills at reviewing front-end.

@JulienPalardJulienPalard merged commit0d781e4 intopython:masterJan 11, 2022
@JulienPalard
Copy link
Member

Thanks@dlukes for the PR !

dlukes reacted with hooray emoji

@dlukes
Copy link
ContributorAuthor

Thank you for the assistance! If you have a minute, I'm just curious -- when can I expect this to land on the live website? Very roughly speaking of course -- next patch release, next minor release? Later than that?

@JulienPalard
Copy link
Member

If you have a minute, I'm just curious -- when can I expect this to land on the live website?

I released python-docs-theme v2022.1 with your commits, so it's just the time to wait for a new build on the backend, which is govenred bya daily cron, so 24h max.

In fact the cron has already picked up some, likefr/3.9 so you can already see it online, 24h being the time for it to be fully deployed on all versions and all languages.

@dlukes
Copy link
ContributorAuthor

Oh wow, great!

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

make sidebar sticky
4 participants
@dlukes@the-knights-who-say-ni@JulienPalard@hugovk

[8]ページ先頭

©2009-2025 Movatter.jp