On Thu, 28 Sep 2023 19:50:21 GMT, Hannes Wallnöfer <hann...@openjdk.org> wrote:

> A few years ago we switched to [Flexbox 
> Layout](https://developer.mozilla.org/en-US/docs/Glossary/Flexbox) to 
> implement the sticky header in the API docs generated by `javadoc`. This 
> solved the problem of anchor link targets [not being positioned 
> correctly](https://bugs.openjdk.org/browse/JDK-8223378), but it also 
> introduced a few new ones:
> 
>  - It required a workaround to get browser history to work (JDK-8249133, 
> JDK-8250779, 8286832)
>  - It changed certain aspects of scrolling behavior in the browser 
> (JDK-8301080)
>  - It changed the way some CSS properties are interpreted (JDK-8315800)
> 
> The reason for most of these problems is that the layout paradigm used by 
> Flexbox is very different from traditional layout of HTML pages. The 
> `scroll-margin-*` CSS properties introduced by the [CSS Scroll Snap 
> Module](https://www.w3.org/TR/css-scroll-snap-1/) provide a simpler and less 
> intrusive way to implement link target offsets in combination with sticky 
> elements implemented using [`position: 
> sticky`](https://developer.mozilla.org/en-US/docs/Web/CSS/position). However, 
> although it is implemented [in all supported 
> browsers](https://caniuse.com/?search=scroll-margin), it comes with its own 
> challanges and quirks, which are explained below.
> 
> The first problem to overcome was that `position: sticky` is more fragile on 
> mobile browsers than Flexbox. If some part of the page content overflows the 
> allotted horizontal space, the whole page can be zoomed out to view the whole 
> content. This causes the header to become unsticky and the link target 
> offsets to become erroneous. It was therefore necessary to make sure page 
> content never overflows its allotted horizontal space, either by adding line 
> break opportunities, or by making all or part of the page horizontally 
> scrollable when its content overflows. Line break opportunities are difficult 
> to add especially in  preformatted code, so I opted for making the parts of 
> the page most likely containing long lines of code scrollable. 
> 
> The next problem was that enabling horizontal scrolling on an element or one 
> of its containing elements breaks the `scroll-margin-top` property in Chrome 
> due to a browser quirk (both desktop and mobile versions). This means that 
> the scrolling must occur in a child element rather than the sections or other 
> elements serving as link targets. 
> 
> When enabling horizontal scrolling on the contents of sections containing 
> user-provided content, another problem is that it disables [Margin 
> Collapse](https://www.joshwc...

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ConstructorWriter.java
 line 109:

> 107:                 currentConstructor = (ExecutableElement)constructor;
> 108:                 Content constructorContent = 
> getConstructorHeaderContent(currentConstructor);
> 109:                 Content scrollBox = 
> HtmlTree.DIV(HtmlStyle.horizontalScroll);

Stylistically, it's a shame to change the name of the variable in this and 
similar situations when all you are changing is its initial value. In the lines 
that follow, its only important that it is some kind of container, not 
specifically a `scrollBox`.  This is the variable name equivalent of whether to 
write

Map m = new HashMap();

or 

HashMap m = new HashMap();

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/resources/script.js
 line 253:

> 251:         history.replaceState(contentDiv.scrollTop, document.title);
> 252:     }
> 253: });

always nice to see workarounds removed!

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/15969#discussion_r1343247028
PR Review Comment: https://git.openjdk.org/jdk/pull/15969#discussion_r1343247547

Reply via email to