docfx icon indicating copy to clipboard operation
docfx copied to clipboard

fix: TOC word-break not being inserted in generic types

Open juliansangillo opened this issue 2 years ago • 2 comments

Expected behavior: Word-break opportunities are inserted into the API references in the TOC. Actual behavior: Word-break opportunities are not inserted if the reference is a generic type, causing that link to overflow its container.

Before fix

before

After fix

after

These were taken from a template I am using. As shown in the before image, the second reference in the TOC on the left word-breaks correctly because it is non-generic, however the fourth and last references is generic and it overflows. I managed to fix it by overriding the docfx.js file in the template and applying the fix there. However, this is just a workaround and isn't an ideal one. It would be better if the fix was included in the default template so overriding this file becomes unnecessary. Also, for some more context, this issue is also present in DocFx's own API documentation site, not just the template I am using. As you can see below, there are quite a few generics in the TOC that overflow.

docfx-site

This issue is caused because the function that inserts the <wbr> (word-break opportunity) tags only does so if this.html() has no html tags and is only text. It does this by comparing this.html() with this.text() to see if they are equal. This only works if the text is a non-generic reference. If it is generic, then this.html() returns the reference with the less-than and greater-than symbols encoded as &lt; and &gt; respectively. this.text() doesn't encode those symbols, so the check fails and the tags are never inserted.

This fix replaces that check. Instead, it checks if this.html() doesn't match this regex: /(<\w*)((\s\/>)|(.*<\/\w*>))/g. This regex matches if there are any html tags. This regex has been tested with https://www.regextester.com/ and the following text:

<p>System.Collections.Generic.List&lt;string&gt;</p>
System.Collections.Generic.List&lt;string&gt;

The first line should match, the second one should not.

juliansangillo avatar Oct 17 '22 01:10 juliansangillo

It seems this (a modification of the docfx.js) should be handled in the custom templates. Some will prefer an automatic horizontal scrollbar to the word-break.

paulushub avatar Oct 17 '22 01:10 paulushub

@paulushub The thing is, word-breaks are already an existing feature. This is just a bug fix for an issue where the word-breaks weren't being inserted for generic classes.

That is an interesting idea though. Although, not everyone may prefer that. I for one like the word breaks as I can see everything without having to scroll. Maybe including scrolling as opposed to word-breaks as an option in globalMetadata could be a feature request for other contributors to tackle?

I also disagree that this should be handled in custom templates. As I stated, this already exists in the default template, it just doesn't work for generics. Also, while I am not an owner of this project, it seems like bad practice to me to override the docfx.js file just to change this. That file has a lot more logic in it. What if any of it changes in future releases of docfx? Then the custom templates won't see the changes.

juliansangillo avatar Oct 19 '22 02:10 juliansangillo