Starting with h3 instead of h2 breaks the page

[goldnead]

@tisonkelley reported in #7

If you don’t include an h2 first on the page, and you just include an h3 it bombs out the whole page, cannot view the page anymore.

[nvkodde]

I believe i'm having the same issue as @tisonkelley. Steps to reproduce:

• first "h" tag on page set as an H3
• later in the content assign text to an H2

When I do that I get an "Undefined array key -1"

If I start with an H2, then an H3 works just fine, but if I use an H1 I get same error. So issue seems to be going up a level from the original value.

[goldnead]

I believe i'm having the same issue as @tisonkelley. Steps to reproduce:

• first "h" tag on page set as an H3
• later in the content assign text to an H2

When I do that I get an "Undefined array key -1"

If I start with an H2, then an H3 works just fine, but if I use an H1 I get same error. So issue seems to be going up a level from the original value.

Thanks @nvkodde, I can reproduce the error now and am working on a fix probably next week. 🙏🏻

[goldnead]

So, after digging into the problem a bit more, I realized, it isn't such an easy fix.

The reason is, that this addon tries to build a nested tree out of the headings you provide. Therefore, the data has to be a valid tree, which has to have a proper starting point.

Imagine the following:

<h2>Level 2</h2>
<h3>Level 3</h3>
<h4>Level 4</h3>
<h1>Level 1</h1>

In this example, when the addon would build a representative nested tree it would look like this:

Level 1
  Level 2
    Level 3
      Level 4

Which wouldn't be in the correct order anymore, as the highest level headline would be the first one as opposed for it being the last one in the provided markup. To solve this, the algorithm tries to build the tree from top to bottom, preserving the original order of headlines. Generally, it does a solid job, when the levels differ as long as there is a clear path it can follow. In the provided example, the last level would be the root level, even though it is the last one, resulting in the reposted error, as it looks for a higher level headline on the first one.

The question is now, how this addon should handle such a situation.
When trying to build a tree, this work and I can't see a solution, where the statamic-internal recursive iteration wouldn't be broken.

1. Throw an error like Invalid semantic headline markup
2. flatten the result and simply don't provide any recursion
3. try to reset the tree when it tries to grab a lower level than the one at the beginning of the tree

In my opinion, the 2nd would be the most easy to implement. Nr. 3 would be the one that could introduce a lot of unexpected behavior.

[nvkodde]

Not sure what is meant in option 3 in regards to resetting the tree, there's probably a balance on developing something that doesn't create too many edge cases.

Generally, the h1 example I would assume is a semantic issue, as you'd expect 1 per page and likely not covered by ToC anyways as it's the title of the article. For the other levels, it seems to be more common to go back/forth, pulled this from Mozilla docs (look at nesting) https://developer.mozilla.org/en-US/docs/Web/HTML/Element/Heading_Elements. So not resetting overall but just ordering the elements as you progress down the page.

[goldnead]

Sure, that's the general aim for this addon. But right now it has its troubles with semantic inconsistencies.
In this example it struggles with the case, when the page starts with a deeper level than one, that is coming after it. So the combination h2 h3 h4 h3 h2 h4 works fine, but as soon as the first heading is surpassed in its level like h3 h4 h5 h2 h3 the addon gets in trouble building a tree from it.
Like, imagine building a <ul> list out of that example. The h2 in the middle wouldn't be possible to represent, as you can't start the multidimensional list from h3 when there is a h2 further down.

A solution could be, to "reset" the tree. Which is probably a confusing term for dynamically adjusting the levels.
Right now, the addon uses the tag name values as levels, so h1 is level 1, h2 is level 2 and so forth which causes the issue.
For a dynamic approach, I can imagine something like this:

Heading 3 -> level 1
  Heading 4 -> level 2
    Heading 5 -> level 3
  Heading 4 -> level2
Heading 2 -> level 1 <--

So, as soon as a heading level is higher than the root level, it creates a new root level.

For this, the addon needs to be rewritten at a few places, but it would make sense to handle such cases properly.

[goldnead]

@nvkodde I pushed a first draft for it into the assigned branch: feature/dynamic-nesting
I'm still not sure if it's not still the best idea to throw an error, since the trees you request to build are "unparseable" on a semantic level...

[el-schneider]

I'm also running into this problem with my clients. I won't pretend I have read all of the above and fully understand the problem, but I can say, that erroring out if clients mess up the order of headings is definitely not ideal, as this is something that will likely happen over and over again. I would much prefer something more graceful, like flattening everything without breaking the page. Funny enough for a while I had though it "fixed" by adding is_flat="true, but this doesn't seem to work anymore (or never has)

[goldnead]

Alright. I'll come up with something. I understand, coming from a clients-perspective, breaking the page because of a semantic error isn't ideal. However, as the error itself is caused by "wrong" semantics due to user input, It will be highly opinionated.
I guess it will come down to logging the error as a warning and suppressing and flattening the output somehow.