BUG: Navigating the Document Tree makes filtered nodes disappear

[bwaidelich]

Description

When a node type filter is active in the navigateComponent, when navigating the node tree previously filtered nodes disappear..

This is not easy to explain, here's a video of the bug in action:

I don't understand what is happening exactly, but I can confirm that this works in the "old UI".

Using Redux DevTools to debug this I can see a lot of removed nodes in the state of the ADD event but I'm not sure if that helps ;)

Background

When there are a lot of (Document) Nodes on one level, this can significantly slow down the NodeTree. For this reason it can make sense to hide certain node types using the Node tree base node type.

[sbruggmann]

Not reproducable in Neos 3.3 with UI 2.x and Neos 4.2 with UI 3.x

[Sebobo]

@bwaidelich do you still get this error?

[bwaidelich]

Yes, unfortunately the issue still exists for me – but not when filtering for node types, but for custom presets.

Steps to reproduce

Add some preset via Settings.yaml:

Neos:
  Neos:
    userInterface:
      navigateComponent:
        nodeTree:
          presets:
            'default':
              baseNodeType: 'Neos.Neos:Document,!Neos.Demo:Chapter'
            'legalPages':
              ui:
                label: 'Test Preset'
              baseNodeType: 'Neos.Demo:Chapter'

...follow the "steps to reproduce" from above

[DrillSergeant]

We start using the custom presets for a current project and now I am affected of this issue.

I could sponsor a bugfix if anyone could do this in the next 2 weeks.

[markusguenther]

As already mentioned in the PR the general issue seems to be resolved but we have some glitches left. So when you move a node inside another node the tree does not show that until you refresh the tree.

Or when you create a new node and the filter is enabled, the new node is also not part of the tree until you refresh the tree. I would say we open a new PR for that. But it works much better than before. Thanks to @dimaip and @DrillSergeant ❤️

Do you want to continue @dimaip or should I start checking that out?

[dimaip]

@markusguenther I can try to fix it on Wednesday probably, but please take over if you feel like you got the time and know how to fix it.

Basically the problem is quite clear: we render all metadata in backend using the default preset. I currently don't know how to fix it. Maybe we need to pass the currently active preset to all backend requests. Maybe there's an easier fix to just mask the issue (like I did in the other bugfix).

[DrillSergeant]

Thanks for the work so far and for caring to fix the last known issue.

[bwaidelich]

Yes, thanks for taking care! Could you elaborate on

Basically the problem is quite clear: we render all metadata in backend using the default preset

Which kind of metadata is that in this case and why is it dependent on the current preset?

[dimaip]

Information about nodes, in particular node children.

[markusguenther]

After a first test in the NodeInfoHelper.php the actions work fine with the legalPages baseNodeType. So passing this information thru the class would be solver :)

So guess will check what we need to change here. And if this will be still a bugfix ;)

[markusguenther]

@dimaip Yesterday I did not find the right angle to tackle that. The quick test in the NodeInfoHelper solved the issue, but passing thru the information is not that easy I guess. As the most endpoints just use the flow query and the controller context.

So maybe adjusting the resulting flow queries are the solution. But did not finish my investigation there yesterday. But in the end the baseNodeType is the root of all eval and we need to adjust that somehow.

Hope to find time soonish... if not maybe you have more luck on Wednesday.

[DrillSergeant]

Would be good to have this done before you do a new UI-release. In our current project the editors (more than 20 of them) will start editing content by the end of next week. It would solve a lot of problem for us if they could use the filter-presets. I can help testing. If sponsoring is needed, please contact me.

[dimaip]

I'm giving it a try now, will keep you posted

[dimaip]

Got it working, will push tomorrow with fresh brain

[bwaidelich]

Unfortunately the bug still exists: When moving pages in a filtered view, the tree is empty after the drag n drop interaction

[pKallert]

This issue still seems to exist when using folders inside a filter. Using Neos Demo (8.3) with the following settings:

Neos:
  Neos:
    userInterface:
      navigateComponent:
        nodeTree:
          loadingDepth: 2
          presets:
            default:
              baseNodeType: 'Neos.Neos:Document,!Neos.Demo:Document.LandingPage'
            global:
              ui:
                label: 'Testing'
                icon: 'fas fa-globe'
              baseNodeType: 'Neos.Demo:Document.LandingPage'

The folders show up, but the items inside the folders are not loading correctly. I made a gif of the behavior:

Also, when first loading the dropdown shows up as open, but I have to click twice to show the subitems.

[grebaldi]

Hi @pKallert,

I'm trying to write up some E2E tests for this, and I cannot reproduce the issue you've described.

But, given your configuration with:

baseNodeType: 'Neos.Demo:Document.LandingPage'

Is the tree even allowed to display the descendants of Features?

I'd interpret the baseNodeType as "show me anything that inherits from this node type". With baseNodeType being set to Neos.Demo:Document.LandingPage, that would exclude all regular pages, or am I mistaken?

[pKallert]

Hi @grebaldi

This was my first time using the filter function, so I am not sure if my expectations werde correct.

I had hoped that the descendants would show up when using the filter since it fits the usecase perfectly. And I was very happy when they did 😄

The plan was to reduce the amount of items in the main tree by moving parts of it into the filter function (everything below folder x moved to filter x). While there are also packages for this, the hope was that it could work with only core logic.

I would argue that if the descendants are shown, they should also work as expected.

As it does only half work right now, I would see two ways this could be resolved:

• not show descendants at all if they do not fit the baseNodeType
• show descendants and fix the loading problem

Of course I would prefer the second one, but it is confusing when the descendants show up on first load and then stop loading when the filter is unset and then reset.

[Sebobo]

Only nodes of the given baseNodeType should be visible and their parents (which can have another type). I'm also setting this up for a customer right now where they need to work only on certain types at a time.

[grebaldi]

@pKallert

As it does only half work right now, I would see two ways this could be resolved:

• not show descendants at all if they do not fit the baseNodeType
• show descendants and fix the loading problem

@Sebobo

Only nodes of the given baseNodeType should be visible and their parents (which can have another type).

Well... I believe @Sebobo's definition fits the original intent of the node tree preset configuration. To achieve a bugfix, we should aim to restore the first behavior as described by @pKallert (so: not show descendants at all if they do not fit the baseNodeType).

To allow for the second behavior described by @pKallert, we would require additional configuration to describe the desired semantics.

Maybe on a per-NodeType basis:

baseNodeType: 'Neos.Demo:Document.LandingPage'
perNodeType:
  'Neos.Demo:Document.LandingPage':
     baseNodeType: 'Neos.Neos:Document' # show any document beneath landing pages

Or on a per-level basis:

baseNodeType: 'Neos.Demo:Document.LandingPage'
perLevel:
  2:
     baseNodeType: 'Neos.Neos:Document' # starting at level 2, show any document 

Or even more flexible:

baseNodeType: 'Neos.Demo:Document.LandingPage'
rules:
   - condition: ${q(ancestors).filter('[instanceof Neos.Demo:Document.LandingPage]').get(0) != null && level > 2}
     baseNodeType: 'Neos.Neos:Document'

Any of these should be doable as a feature addition for 8.4.

Wdyt?

[pKallert]

@grebaldi sorry for the late answer 🥲

Doing a bugfix & new feature sounds good!

I think option 1 would be best, per-level is too restricting when using multiple baseNodeTypes and I am kind of afraid of what issues user could run into when using option 3.

I am not sure about the perNodeType naming, perhaps something like DescendantsPerNodeType?

[grebaldi]

@pKallert:

Doing a bugfix & new feature sounds good!

Great :) I believe that the remaining bug behavior is covered by the fix in https://github.com/neos/neos-ui/pull/3822 (Correct me, if I'm wrong). So, we could add this issue to the list and let it be closed, once #3822 is merged.

For the feature part, I think it'd be best to start a fresh issue (with reference to this one) and specify the desired behavior there.

Since you're close to a specific use-case, would you like to write the spec, @pKallert?