neos / neos-ui

Neos CMS UI written in ReactJS with Immutable data structures.
GNU General Public License v3.0
265 stars 134 forks source link

BUG: Navigating the Document Tree makes filtered nodes disappear #1691

Open bwaidelich opened 6 years ago

bwaidelich commented 6 years ago

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:

broken_node_filter

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 ;)

image

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 commented 5 years ago

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

Sebobo commented 5 years ago

@bwaidelich do you still get this error?

bwaidelich commented 5 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

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

bwaidelich commented 3 years ago

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 commented 3 years ago

Information about nodes, in particular node children.

markusguenther commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

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

dimaip commented 3 years ago

Got it working, will push tomorrow with fresh brain

bwaidelich commented 3 years ago

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

pKallert commented 4 months ago

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:

ezgif-2-dbd21c7915

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

grebaldi commented 2 days ago

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 commented 1 day ago

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:

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 commented 1 day ago

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 commented 1 day ago

@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?