✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

_{Generated for commit: 6b54800. Link to the linter CI: here}

On my screen, this change makes the section title hidden by the top bar of the website

This PR:

Main:

I can't really see what changes except for the spacing above the "HDBSCAN" heading being a bit bigger. Is this the change you mean?

#26625 added more margin to all the section headers. I updated this PR to remove the margins. With this PR, the anchor improvement from #25783 still works. Here is the release highlights for 1.2:

Main

PR

I'm guessing the intention of #26625 was to add more white-space above sections in the body. With 21be8be (#26787), I reintroduced the white-space:

The spacing above section is not a bug but a feature. Empty space is really important for readability of documents.

As a reference, see for instance the bootstrap documentation (eg https://getbootstrap.com/docs/5.3/getting-started/introduction/ ).

Bootstrap people know a lot about UX and readability.

In general I agree with "adding empty space helps readability". It is the number one feature to use if you need to predict if a layout was done by a professional or an amateur: look at how much empty space there is. The next feature being: is the spacing asymmetric between the top and the bottom (pros leave more space below than above).

However, I think we should not spend our time arguing about this. So for me the question is: is this PR changing something that was previously changed on purpose or is it changing something which changed as a side-effect? If the former, I'd vote to leave it as is. If the latter, then lets merge this PR.

The reason I think we are better of not arguing (and because none of us are experts I'd claim we are arguing not discussing ;)) is that I think something like #26809 could bring a lot of benefits in terms of readability, layout, reduced "fuzzing with HTML and CSS", accessibility, etc. This means we should invest our energy there, because it is going to be a big job.

The reason I think we are better of not arguing (and because none of us are experts I'd claim we are arguing not discussing ;)) is that I think something like #26809 could bring a lot of benefits in terms of readability, layout, reduced "fuzzing with HTML and CSS", accessibility, etc.

Transitioning a theme well is harder than it looks

https://output.circle-artifacts.com/output/job/b11ccb31-7f7e-44e7-8ae2-f591cbd412fe/artifacts/0/doc/modules/generated/sklearn.ensemble.HistGradientBoostingRegressor.html#sklearn.ensemble.HistGradientBoostingRegressor is the equivalent to the link on main, but with these changes.

For me the two (main and PR) look the same :-/

The element that is highlighted in the screenshot is the one that the fragment in the URL refers to. At least I think so. I think the browser (Firefox in this case) is doing the correct thing, it made sure the top of the element is within the viewing area but, there is something on top of it (the menu bar). I think the only way to fix this is to "make the element bigger" so that having the top of it in the viewable area means the actual content is far enough away from the top so that the menu doesn't cover it.

Maybe there is a way to tell the browser that the menu is covering up the element and that it needs to scroll more to make it visible?

I don't know how to "make the element bigger" :-/ Adding margin to it doesn't help and we don't want to increase the padding.

What does seem to "just work" is https://output.circle-artifacts.com/output/job/b11ccb31-7f7e-44e7-8ae2-f591cbd412fe/artifacts/0/doc/modules/generated/sklearn.ensemble.HistGradientBoostingRegressor.html#sklearn-ensemble-histgradientboostingregressor (dashes in the ID instead of dots). Maybe the solution is to link to that instead?

Joy, HTML and CSS trickery :D

I updated this PR to be simpler. It only removes the space about the H1 tag. Now the whitespace is reduced instead of removed. I agree that whitespace is useful, but the H1 header feels weird to not be aligned with the sidebar. The bootstrap docs site does not have the solid blocks that we do.

main

PR (Does not apply anymore)

As a compromise, I updated the PR again to reduce the whitespace in the H1 tag. This is around the same space between the actual text and the navbar as the bootstrap site

Hi @thomasjpfan
I would argue that the whitespace of the h1 is the most important, to separate it from the text above and make it more visible.

If you really feel bad about the space above the first h1 which makes that it is no longer aligned with the top of the page, I would suggest to use the ":first-of-type" to reduce the space only for the first h1.

Another, maybe even better option, would be to add vertical only on h1 that are "general siblings" of the enclosing div, using the "~" selector, which will not apply to the h1 right are the beginning of the div.

I would however argue that even on the first h1, a bit of vertical space is good.

I sent a PR here thomasjpfan#121 to suggest a way to go in your direction

I believe that this is ready for merge. @betatim , do you want to have a last look? Without reply from you in a few days, I will merge, as I think that you were OK with the changes.

I'm merging this: I worry that if we wait too long it is going to get lost in the weeds (I myself had a hard time finding it amongst all the opened PRs).