Fix JS errors in doxygen navtree#6326
Conversation
|
This needs more testing before being merged to make sure the behavior is correct, but it should fix anchor links in some browsers and allow the navtree to exist alongside the created TOC. Sample page before: After: |
| @@ -1,3 +1,524 @@ | |||
| /* | |||
| @licstart The following is the entire license notice for the JavaScript code in this file. | |||
There was a problem hiding this comment.
Most of the functions that were being overridden before are nested functions now, so we're effectively just modifying the original code. Included the license in this file.
However, the note below of "The above is the entire license notice for the JavaScript code in this file" is not necessarily the case for code that we added, so it may need to be put in a separate file.
There was a problem hiding this comment.
Do you mean that we need to add an additional license due to us adding code, or that because we've modified parts of this code, the license should be moved? From reading the license information, it seems like the license holds even with modifications to the code.
There was a problem hiding this comment.
I think it's a bit ambiguous since previously we were just overriding the original code with our own functions without having directly modified the original code. It's possible that some of those modifications, such as the new functions we added, could have been covered under our BSD license, but the rest of the code was similar to the original code from what I could tell, so that may have been more appropriately covered under the missing MIT license.
Since then, the functions that we were overriding have been moved to being nested functions, meaning that overriding them won't work as intended and we now have to modify the original code. The same new functions we added could be covered under the MIT license or our BSD license.
There was a problem hiding this comment.
The vast majority of the changes in this file consist of just copying over navtree.js from a build directory and modifying just a few spots from the previous logic.
| domNode.appendChild(node.expandToggle); | ||
| // Each item in the nav tree is currently 30px. This may change, but | ||
| // this knowledge is needed to get a reasonable size for the TOC. | ||
| if (NAVTREE) { |
There was a problem hiding this comment.
There were many cases where the height of the TOC was larger than the calculated window height, which would cause weirdness with resizing. Doxygen appears to give us navtreedata.js, which contains this variable to tell us how many items are in the navtree. Given that those items are 30px in height currently, this gives a reasonable limit on the size of the navtree. This is somewhat fragile, but the most practical way I could think of to determine how to size the navtree against the TOC.
| // Hide the root node "HDF5" | ||
| $(document.getElementsByClassName('index.html')[0]).parent().parent().css({display:"none"}); | ||
|
|
||
| resizeHeight(); |
There was a problem hiding this comment.
Needed to make sure things get resized at an appropriate time.
jhendersonHDF
requested review from
bmribler,
brtnfld,
derobins,
fortnern,
glennsong09,
lrknox,
mattjala,
qkoziol and
vchoi-hdfgroup
as code owners
| a.href = srcPage!=targetPage ? url : aname; | ||
| a.onclick = function() { | ||
| storeLink(link); | ||
| aPPar = $(a).parent().parent(); |
There was a problem hiding this comment.
This creates an accidental global. Should be const aPPar = ....
|
The top tabs already provide the global navigation (Getting Started, User Guide, Reference Manual, Cookbook, etc.). So the sidebar navtree is duplicating that. |
Im happy to simply have the TOC take up all the space and overwrite the navtree, but then the question becomes what point is there in having the navtree? |
|
Using the PAGE_OUTLINE_PANEL from Doxygen 14+ looks like below. Seems like this might be preferable to trying to hack our own together. 
|
jhendersonHDF
force-pushed
the
fix_doxygen_navtree
branch
from
9a1d0e9 to
6a8f6a8
Compare
Fixes anchor links in some browsers and restores the navtree alongside the Table of Contents for several pages
|
This draft has had no activity for 60 days and has been marked
Checking the box is the only thing that resets this -- a commit or other automated update alone won't. Otherwise it will be flagged for maintainer review. |
github-actions
Bot
added
draft-stale
|
Thanks for confirming — removing the stale label. |
jhendersonHDF
force-pushed
the
fix_doxygen_navtree
branch
from
6a8f6a8 to
87036da
Compare
Review ChecklistThis PR touches the following areas. Each needs a sign-off
Additional reviewers (not owners of a touched area): @fortnern, @bmribler, @mattjala |
6 participants
Fixes anchor links in some browsers and restores the navtree alongside the Table of Contents for several pages