On Wed, 16 Jul 2025 13:05:03 GMT, Nizar Benalla <nbena...@openjdk.org> wrote:

>> Please review this patch to add a toggle to order the member details in the 
>> table of contents in lexical order. The selected choice is stored and 
>> preserved.
>> 
>> Here is a preview of the new toggle.
>> 
>> 
>> https://github.com/user-attachments/assets/55c81e4b-5fc0-416e-8946-53aede419640
>
> Nizar Benalla has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   make toc.js resilient to being loaded after script.js

This looks good, but there's some room for improvement. 

My biggest gripe is that it only reorders the members in the table of contents, 
but not the member details in the page. Thus, when scrolling through the page, 
the selection in the TOC jumps around randomly. The idea of the 
scroll-following TOC is that it mirrors the order of page contents, so both TOC 
and page contents should be reordered IMO.

I think the icon is not very good at hinting at its function. Some variant of  
[A-Z sort icon ](https://www.flaticon.com/free-icons/sort-alphabetically) might 
work better. There should probably be a different icon for non-lexicographic 
source order, but I'm not sure what that should be. I can help with creating 
the icons.

Is there a reason for putting the functionality in a new JavaScript file? I 
agree `script.js` is a bit overloaded and unorderly, but I would prefer 
cleaning up that existing file instead of adding a new one.

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

PR Comment: https://git.openjdk.org/jdk/pull/26322#issuecomment-3082983826

Reply via email to