Fixes function searching and jumping #389
Conversation
✅ Deploy Preview for grist-help-preview ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
Thank you @Spoffy ! There is another piece of behavior that seems to need fixing: when clicking on a link to go to a function, we used to have a bit of JS that expands that function (since normally all are collapsed). It is in |
Thanks for pointing me at the right file - this should be all sorted now too! |
help/en/docs/css/grist.css
Outdated
| display: none; | ||
| } | ||
|
|
||
| /* The <code> block that serves as a function header annoyingly gets auto-wrapped in a <p> */ |
There was a problem hiding this comment.
Is this comment out-of-date / no-longer-helpful now? The comment before the previous change also seems in need of an update.
There was a problem hiding this comment.
Updated them to better reflect their uses!
help/en/docs/js/grist.js
Outdated
| if (elem && elem.tagName === 'DETAILS') { | ||
| for (var el of document.querySelectorAll('details')) { | ||
| el.open = (el === elem); | ||
| var elemToTest = elem; |
There was a problem hiding this comment.
Would this be enough to replace the loop: elemToExpand = elem.closest('DETAILS')?
There was a problem hiding this comment.
Aha, should be! Didn't know that function existed. :)
help/en/docs/js/grist.js
Outdated
| if (elem && elem.tagName === 'DETAILS') { | ||
| for (var el of document.querySelectorAll('details')) { | ||
| el.open = (el === elem); | ||
| var elemToTest = elem; |
There was a problem hiding this comment.
Stylistically, there's no need to use var instead of modern let/const, is there? I know it's already there, but we don't need to keep using var for new code.
There was a problem hiding this comment.
I was on the fence on this one myself, and figured I'd stick with the current approach for now.
In terms of compatibility - let/const are at 97%, so doubt there's any issues using them really.
This PR: