jimmynotjim / scrollnav

A dependency free JavaScript plugin for auto generating single page navigation
http://scrollnav.com
MIT License
461 stars 127 forks source link

Created sections don't wrap plain text blocks (text without wrapping tags) #48

Closed batspy closed 10 years ago

batspy commented 10 years ago

Hi,

If we try to create a navigation for this part of a paragraph,


<section>
            <h2>Description</h2>
            scrollNav is a light jQuery plugin that grabs your page's existing content, divides it up into logical sections and builds a customizable scrolling sidebar navigation. Scroll this page and watch the nav follow along with you.
            <p>test test test test test test test test test test test test test test test test test test test </p>
</section>

then the resulting section will skip the following text: scrollNav is a light jQuery plugin that grabs your page's existing content, divides it up into logical sections and builds a customizable scrolling sidebar navigation. Scroll this page and watch the nav follow along with you.

It will only keep the elements that are wrapped in some html element.

jimmynotjim commented 10 years ago

Can you clarify? I don't understand the question here.

batspy commented 10 years ago

The point is that your script contains the folloing code:

raw_html.push( $(this).nextUntil(target_elems).andSelf() );

but nextUntil will only search for html elements not text blocks inside, so it will skip them.. In the above example the result html would have a new scetion with the h2 tag and p (paragraph) tag and it will be closed with just these. After the section's closing tag all the rest text block will be printed bellow.

So as a result we would have sth like this:


<section class="scroll-nav__section" id="scrollNav-1">
<section>
            <h2>Description</h2>
            <p>test test test test test test test test test test test test test test test test test test test </p>
</section>
</section>
scrollNav is a light jQuery plugin that grabs your page's existing content, divides it up into logical sections and builds a customizable scrolling sidebar navigation. Scroll this page and watch the nav follow along with you.

jimmynotjim commented 10 years ago

Ah, I understand now. Created a test and confirmed the issue.

http://jsfiddle.net/jimmynotjim/vpurjbhm/

Not sure when I'll have time to address this, but feel free to send a PR if you've already got a fix for it.

jimmynotjim commented 10 years ago

After searching everywhere I can think of, it seems there's no solution I can add to the plugin. Due to the way jQuery traverses the DOM, there's no way for it to correctly capture your text block.

Although a plain text block is valid HTML, it's still recommended to wrap it in an element. Without an element there's zero context to the content and it lacks semantics.

batspy commented 10 years ago

I understand and all you have said are correct, but when there is a valid html there is also a problem using it.. I am thinking sth like string manipulation of the html block wrapping up all text blocks before even using them..

jimmynotjim commented 10 years ago

Feel free to send a PR with tests if you can find a way to make it work, but at this point it's outside of what's possible with jQuery DOM traversal, so it's out of scope of the plugin.