DOC add permalink to summary of collapsed details section#27409
DOC add permalink to summary of collapsed details section#27409glemaitre merged 15 commits intoscikit-learn:mainfrom
Conversation
|
I am missing the auto-scrolling to the permalink when accessing the link. |
|
Of course, I forgot to add the |
|
It remains 1 annoying thing that is already present in the website: the referencing will place the anchor at position 0 px and it will be hidden under the navbar. |
|
I opened #27418 without realizing you had worked on this. Should I close it? |
|
OK, so the results are now compelling regarding the rendering. The only drawback is the need to add create manually the referencing that was done by Sphinx in the past. I don't know how many of such editing we will need. Also, in the future, these links will not be checked by Sphinx and we can get dead link if the documentation page structure is changing. |
This is reverting the problem but I assume that we will encounter it again on some other part. If this is really meaningful to have I think that we should come with a solution. |
|
I am not a fan of the extra complexity induced by this PR. I think I would much prefer the approach of #27418. |
|
Having the permalinks on the details headers is nice though but I would still avoid breaking sphinx references. |
|
Just a tweak: #27418 introduced a typo "shortand" -> "shorthand" which I was too slow to catch. Now that you're working on this, can you please take care of it? |
Indeed, I did solve the problem in |
|
I know this is not purely documentation since there is a little bit of code but I think @scikit-learn/documentation-team you can have a review and merge this one if the rendering is what you expected from the original issue. |
| // The ID uses the first line, lowercased, and spaces replaced with dashes | ||
| var anchorID = summaryElement.textContent.trim().split("\n")[0].replace(/\s+/g, '-').toLowerCase(); | ||
| detailsElement.setAttribute('id', anchorID); |
There was a problem hiding this comment.
Question out of ignorance, what happens if the summary text contains special character such as \/:*? "<>|&%. '#[]+=„ ?
Or can we rather use .replace(/[\s\/:*?"<>|&%. '#\[\]+=„]+/g, '')?
There was a problem hiding this comment.
I think there might be cases where the summary text is repeated across a file, such as References or similar. Is there a way to add a counter of appearance per file or allow for custom anchor in some cases?
There was a problem hiding this comment.
Regarding the special characters, I think that they can be part of the URL and it should not be a problem. I would not address this issue now. We can always change the code if in the future we have a compeling example.
For the second suggestion, this is something that I did not think about and this is a really good point. I will initiate a counter add it as a suffix.
ArturoAmorQ
left a comment
There was a problem hiding this comment.
The rendering looks good so I am approving this PR, but this is definitely not my field of expertise.
lucyleeow
left a comment
There was a problem hiding this comment.
This is also outside of my wheelhouse but the code is well commented and it works well for me.
…rn#27409) Co-authored-by: Arturo Amor <86408019+ArturoAmorQ@users.noreply.github.com>
…rn#27409) Co-authored-by: Arturo Amor <86408019+ArturoAmorQ@users.noreply.github.com>
closes #27127
follow-up of #26872
Add permalink to summaries of collapsed details section.