search

tscanlin / tocbot

Build a table of contents from headings in an HTML document.
MIT License
1.39k stars 115 forks source link

fix passing a DOM element into init #267

Closed BenceSzalai closed 2 years ago

BenceSzalai commented 2 years ago

tocElement option only works if tocSelector selects the same element

See: https://github.com/tscanlin/tocbot/issues/266

tscanlin commented 2 years ago

Hey, thank you for the PR / fix and the test updates! Looks good. 👍

tscanlin commented 2 years ago

Hmm, locally it seems to have an issue for me when selecting certain headings. (like methods in the example npm start since the scroll height is small). I'm not sure why though..

BenceSzalai commented 2 years ago

I'm happy to debug, if you could let me know how to replicate the issue!

tscanlin commented 2 years ago

Thanks! To reproduce, you can run the dev / docs site locally (with npm start) and click any of the headings in the Toc under API > Methods, note that clicking them in the toc doesn't seem to scroll to the right place.

But here it doesn't have that issue: http://tscanlin.github.io/tocbot/

BenceSzalai commented 2 years ago

Okay, it looks like I've unintentionally introduced an issue.

While I was looking at the code I've noticed this line:

if (obj !== document.querySelector(options.contentSelector && obj != null)) {

and I just thought to myself this cannot be right, as for why would anyone want to call document.querySelector with an argument that is a boolean as options.contentSelector && obj != null would always either be true or false. Which is fine, actually this line is wrong, but it is wrong in a way that it is good after all, because all that is really needed is this:

if (obj !== null) {

Which after all it is since document.querySelector(options.contentSelector && obj != null) is always null.

But I've for some reason assumed the intention was rather this:

if (obj !== document.querySelector(options.contentSelector) && obj != null) {

So my mistake was to assume an intent without really looking at what was needed.

It is fixed now, I'll submit a PR.