Recursing into a navigation item causes links and labels to mutate

[tzbarkan]

Which exact Umbraco version are you using? For example: 8.7.0 - don't just write v8 or v9

8.16.0

Bug summary

Given this structure in the property editor:

• Label A
  • Label B
  • Link X
  • Link Y (with target = _blank)
  • Link Z (with custom class .whut)
  • Label C

And this helper:

@helper makeNav(IEnumerable<UmbNavItem> items){
  <ul>
    @foreach (var item in items) {
      <li>
        @if (item.ItemType =-- UmbNavItemType.Label) {
          <h3>@item.Title</h3>
        }
        else {
          @item.GetLinkHtml()
          <a href="@item.Url()@item.Anchor" class="@item.CustomClasses" target="@item.Target">@item.Title</a>
        }

        @makeNav(item.Children)
      </li>
    }
  </ul>
}

Executing the helper:

1. Label A remains a label and renders as expected
2. Labels B & C become Links, with no link properties, rendering as <a>Label B/C</a>
3. Link X remains a link, but getLinkHtml() outputs nothing
4. The manually constructed <a> tag for Link X has an href, but @item.Title is empty
5. Links Y & Z remain links and keep value for @title and getLinkHtml() still works as expected

Specifics

No response

Steps to reproduce

1. Create Label A
2. Create Label B
3. Create Link X and drag it as child of Label B
4. Create Link Y with target _blank and drag as child of Label B
5. Create Link Z with custom class and drag as child of Label B
6. Create Label C and drag as child of Label B
7. Then drag label B as child of Label A

Execute helper with recursion.

Expected result / actual result

1. Links should remain link elements with all properties the same as at higher levels.
2. Labels should remain label elements as at first level

[tzbarkan]

FYI: this bug is a blocker for us. If it's not resolved soon, we have to fall back to meganav. No pressure, just curious: what's your prognosis about fixing this one?

[AaronSadlerUK]

It's on my list, labels don't exist in MegaNav, I added them as part of the UmbNav migration.

Happy to recieve PRs if things are needed quickly, this is something i support in my spare time for free so all help is appreciated 😇

[AaronSadlerUK]

Following your instructions this is the output I got:

<ul>
    <li>
        <h3>Label A</h3>
        <ul>
            <li>
                <a href="">Label B</a>
                <a>Label B</a>
                <ul>
                    <li>
                        <a href="#linkx">Link X</a>
                        <a href="#linkx#linkx">Link X</a>
                    </li>
                    <li>
                        <a href="#linky" target="_blank">Link Y</a>
                       <a href="#linky#linky" target="_blank">Link Y</a>
                    </li>
                    <li>
                        <a class="linkzclass" href="#linkz">Link Z</a>
                        <a href="#linkz#linkz" class="linkzclass">Link Z</a>
                    </li>
                </ul>
            </li>
            <li>
                <a href="">Label C</a>
                 <a>Label C</a>
            </li>
        </ul>
    </li>
</ul>

From what I can see, the only issue is with the label being output as a link, this is as expected as you are calling @item.GetLinkHtml(), I agree this should be handled better as per #16.

[tzbarkan]

Here's the literal code I'm using. ANd we still see this on RC10:

@helper BuildSubNavItems(IEnumerable<UmbNavItem> items, int level = 0){
    const int maxDepth = 2;
    int maxItems = 4;

    <ul class="umbnav__list umbnav__list--lvl-@level" data-navlevel="@level" data-navchildren="@items.Count()">
        @foreach (var item in items.Take(maxItems))
        {
            <li class="umbnav__item umbnav__item--lvl-@level @item.CustomClasses">
                @* TO-DO: add ability to do images later. Model will need to be updated, too *@
                @if (String.IsNullOrEmpty(item.Url())) // Hack because it will give type of Link
                {
                    <div tabindex="0" class="umbnav__label @item.CustomClasses">@(String.IsNullOrEmpty(item.Title) ? "Mutated" : item.Title)</div>
                }
                else
                {
                    @item.getLinkHtml()
                    <a href="@item.Url()@item.Anchor" class="@item.CustomClasses" target="@item.Target">@(String.IsNullOrEmpty(item.Title) ? "Mutated" : item.Title)</a>
                }

                @BuildSubNavItems(item.Children, level + 1)
            </li>
        }
    </ul>
}

I tried adding a custom class to every item in the menu. This worked to force all labels to stay labels on recursion. However, it has intermittent results on links. I can't identify the pattern yet. Here, all of the items have a custom class, but for some reason, only one of them remains a link on recursion at level 2.

[tzbarkan]

With the above code, The output looks like this for Madre de 2. (I need to check that children isn't 0, oops):

I replace URLs with (correct) to make this more readable.

<li class="umbnav__item umbnav__item--lvl-0 avoid-mutation">
  <div tabindex="0" class="umbnav__label avoid-mutation">Madre de 2</div>
  <ul class="umbnav__list umbnav__list--lvl-1" data-navlevel="1" data-navchildren="2">
    <li class="umbnav__item umbnav__item--lvl-1 avoid-mutation">
      <div tabindex="0" class="umbnav__label avoid-mutation">Nivel 1</div>
      <ul class="umbnav__list umbnav__list--lvl-2" data-navlevel="2" data-navchildren="5">
        <li class="umbnav__item umbnav__item--lvl-2 avoid-mutation">
          <a class="avoid-mutation" href="(correct)"></a>
          <a href="(correct)" class="avoid-mutation">Mutated</a>
          <ul class="umbnav__list umbnav__list--lvl-3" data-navlevel="3" data-navchildren="0">
          </ul>
        </li>
        <li class="umbnav__item umbnav__item--lvl-2 avoid-mutation">
          <a class="avoid-mutation" href="(correct)"></a>
          <a href="(correct)" class="avoid-mutation">Mutated</a>
          <ul class="umbnav__list umbnav__list--lvl-3" data-navlevel="3" data-navchildren="0">
          </ul>
        </li>
        <li class="umbnav__item umbnav__item--lvl-2 avoid-mutation">
          <a class="avoid-mutation" href="(correct)"></a>          <a href="(correct)" class="avoid-mutation">Mutated</a>
          <ul class="umbnav__list umbnav__list--lvl-3" data-navlevel="3" data-navchildren="0">
          </ul>
        </li>
        <li class="umbnav__item umbnav__item--lvl-2 avoid-mutation test-class">
          <a class="avoid-mutation test-class" href="(correct)">Planes (con una clase)</a>          <a href="(correct)" class="avoid-mutation test-class">Planes (con una clase)</a>
          <ul class="umbnav__list umbnav__list--lvl-3" data-navlevel="3" data-navchildren="0">
          </ul>
        </li>
        <li class="umbnav__item umbnav__item--lvl-2 avoid-mutation">
          <div tabindex="0" class="umbnav__label avoid-mutation">No pongas etiquetas aquí</div>
          <ul class="umbnav__list umbnav__list--lvl-3" data-navlevel="3" data-navchildren="0">
          </ul>
        </li>
      </ul>
    </li>
    <li class="umbnav__item umbnav__item--lvl-1 avoid-mutation custom-label">
      <div tabindex="0" class="umbnav__label avoid-mutation custom-label">Nivel 1 (con una clase)</div>
      <ul class="umbnav__list umbnav__list--lvl-2" data-navlevel="2" data-navchildren="4">
        <li class="umbnav__item umbnav__item--lvl-2 avoid-mutation">
          <a class="avoid-mutation" href="(correct)"></a>          <a href="(correct)" class="avoid-mutation">Mutated</a>
          <ul class="umbnav__list umbnav__list--lvl-3" data-navlevel="3" data-navchildren="0">
          </ul>
        </li>
        <li class="umbnav__item umbnav__item--lvl-2 avoid-mutation">
          <a class="avoid-mutation" href="(correct)"></a>          <a href="(correct)" class="avoid-mutation">Mutated</a>
          <ul class="umbnav__list umbnav__list--lvl-3" data-navlevel="3" data-navchildren="0">
          </ul>
        </li>
        <li class="umbnav__item umbnav__item--lvl-2 avoid-mutation">
          <a class="avoid-mutation" href="(correct)"></a>          <a href="(correct)" class="avoid-mutation">Mutated</a>
          <ul class="umbnav__list umbnav__list--lvl-3" data-navlevel="3" data-navchildren="0">
          </ul>
        </li>
        <li class="umbnav__item umbnav__item--lvl-2 avoid-mutation">
          <a class="avoid-mutation" href="(correct)"></a>          <a href="(correct)" class="avoid-mutation">Mutated</a>
          <ul class="umbnav__list umbnav__list--lvl-3" data-navlevel="3" data-navchildren="0">
          </ul>
        </li>
      </ul>
    </li>
  </ul>
</li>

[AaronSadlerUK]

When a link is entered into the backoffice it's stored in the property json, and as such is stored within the Umbraco cache.

When a node is chosen in the backoffice this is also stored in the property json, however the model get's the latest information from the cache at render using either the Guid / Id or Udi depending on what's stored.

At this point I can only come to the conclusion that this is a cache issue within the site, and my suggestion is to rebuild the indexes and clear out the TempData folder within AppData.

Failing that as a last resort rebuilding the menu in the backoffice by removing all links, save and publish and then adding the links back in again will create a fresh json blob.

[tzbarkan]

Will try

[tzbarkan]

FYI: I deleted app_data, cleared, the cache, and built a new test menu from scratch. The mutation still occurs for us as above.

We are using a Multi Site setup, which might affect trying to duplicate the error.

[AaronSadlerUK]

Hmmm ok I don't have a multisite test site currently.

I do not have enough time at the moment to create a multisite test site and look into this any further, do please fork the source and if you manage to fix this submit a PR.

I have not had any issues on the multisite setups I have used this on recently.

[tzbarkan]

One more question on this and I'll stop being annoying: I just realized that maybe you built this without intending labels to have link children. But perhaps the new feature will be a workaround for it?

[AaronSadlerUK]

Labels and links should be fully interchangable, and you should also be able to change a link into a label, or a label into a link.

You should also be able to add links and labels as many levels nested below each item.

[AaronSadlerUK]

If you could provide a site mirroring your setup, using a starterkit that can demo this issue in a way I can debug it, then that would be useful.

[tzbarkan]

Installing RC11 fixed the mutation.

[AaronSadlerUK]

🤯 I wonder what the cause was 🤔

I did fix an issue with the itemType being incorrectly set, so maybe it was that.

Great news though!

[tzbarkan]

I celebrated too soon: rc11 works with menus made with rc10, but it there are new issues for us. Creating a new menu in rc11 with labels, or editing an existing menu with labels causes the entire item to have zero children. Now even top level labels are gone. rc11 only works for us if the menu is made completely of links. (I haven't tried images).

The new display-as-label feature bypasses this bug in our setup.

I know I need to give you a way to replicate, but just documenting the info here in case it becomes useful. :)

[AaronSadlerUK]

Thats strange, my thoughts are it must be due to the recursive call in the builder service.

Once i have something to test with it would be good to resolve this