BUGFIX: fix hover on toggle icon not changing color to blue

[Devclaim]

What I did

Problem 1: Hover States

Looking at the CSS it looks as if the toggle button of a node in the Content and Page NodeTrees is supposed to change their color to blue when hovering over them.

While at it I also replicated the behavior from the top left burger menu. The Label and the icon now appear blue when they are either hovered over or selected (While selected the grey Background is still present).

Problem 2: Collapsing all children

Since I am new to Reselect I wasn't able to create a new selector to get all descendants of a node. Also I didn't want to break anything accidentally. Instead I added the function to optionally collapse the children of a node by shift-clicking the toggle icon of a node.

Problem 3: Make it easier to drop a node into a dropTarget

How I did it

Problem 1: Hover States

• Styles are now applied to the toggle SVG
• Added a hover state to the nodes

Problem 2: Collapsing all children

1. The toggle action now has more arguments

   • collapseChildren: boolean true if the shift key is pressed on the click event
   • childrenContextPaths: NodeContextPath[] the context paths of all children of the clicked node
     • children that cant be toggled are filtered out
     • checks if children are loaded at all
   • childrenCollapsedByDefault: boolean
     • checks if children are collapsedbydefault (unlike a normal node these nodes must be removed from the redux store to be appear collapsed

2. To integrate a simple collapse all button the toggle buttons could be selected and the click events could be fired including the shiftKey event via simple JS. I wonder if a solution like this would be up to the standards? Since the redux actions would be fired, I don't think it would create any unwanted side effects.

Problem 3: Make it easier to drop a node into a dropTarget

I added extra height to the dropTarget but only after a node is already being dragged.

How to verify it

Before: No hover effects

After:
New Hover Effects and collapsing children nodes in action:

[markusguenther]

Thanks for providing this PR. I tried to find out since when it is not working and the oldest Neos I could find on my computer was a 4.3. There it did not hover. So maybe it was a feature of the old not React based UI.

[markusguenther]

Finally, tested the PR and it looks really excellent. Sadly, it feels more like a feature than a bug fix. The styling with the hover is like a bug fix, but that whole change of the look and feel looks to me like a feature.

What do the others think @mhsdesign @Sebobo @grebaldi

new-drag-and-drop.mp4

[markusguenther]

From my personal point of view, I would bring that to the 8.4 branch, then the latest LTS will get the new look & feel and as it was never in the React based UI with the hover effect, I bet that it does not hurt when we skip 7.3 here. But that's my personal viewpoint. We will see what the others think, but I like the look & feel. Thank you 🙏

[manuelmeister]

This is a bugfix from an accessibility standpoint.
From a design perspective, I'm not sure that the hover color should be the same as the active color. It may be better to use the lighter background instead of the text color to indicate the hover/focus.

Additionally, it would be beneficial to target keyboard focus, not just hover.

[Sebobo]

Fully agree with Manuel.

Btw. @Devclaim thx for your first PR and the great change!

[markusguenther]

This is a bugfix from an accessibility standpoint. From a design perspective, I'm not sure that the hover color should be the same as the active color. It may be better to use the lighter background instead of the text color to indicate the hover/focus.

Additionally, it would be beneficial to target keyboard focus, not just hover.

Thanks for the input @manuelmeister a person you should know well also suggested an improvement for the tree.
Sadly, it did not make it yet to a PR. It would be nice to have an accessible tree. I started once but did not make the fast progress I expected to do ;)

#2938

[markusguenther]

@manuelmeister @Sebobo I changed the styles a bit. The focus adjustment makes no sense currently, as the tree is inaccessible via Keyboard at all. So here we need more work and that would target 8.4 then I guess.

new-drag-and-drop.-.01.mp4

[Devclaim]

Heya, I pulled and tested the changes by @markusguenther and I like them. There is also one more thing: technically collapsing children nodes via shift-leftclick is a "feature". Should I remove that part then or can it stay in this PR ?

[grebaldi]

Hi @Devclaim,

thanks a lot for your first contribution! :)

I've tested your change with a local setup, and here's what I observed:

• ✔️ Text, Icon and background of a tree node change color on hover
• ✔️ Toggle-Chevron of a toggable tree node changes color on hover
  • I agree with your assessment, that this was always supposed to be the case, as indicated by

    neos-ui/packages/react-ui-components/src/Tree/node.css

    Lines 21 to 23 in dbb102a

    	&:hover {
    	color: var(--colors-PrimaryBlue);
    	}

• ❌ Height of drop target increases when a node / a selection of nodes is dragged.
  • see my code comment below
• ✔️ Shift + Left-Click collapses the children of a node

I like the style changes, and definitely give my 👍 for those.

To effectively increase the height of the drop targets, additional adjustments would be needed. I'd like to suggest a separate PR for that (can be counted as BUGFIX as well, for the same reason that @manuelmeister mentioned).

Regarding the Shift + Left-Click-feature, you asked:

technically collapsing children nodes via shift-leftclick is a "feature". Should I remove that part then or can it stay in this PR ?

Imho, there are several problems here:

1. The current implementation is a breaking change
   I give a more elaborate explanation for this in my code comment below
2. Shift + Left-Click doesn't strike me as a good solution to capture the user's intention to collapse the children of a node. It seems very obscure and uncommon to me (I've never seen a tree component behave like this - but then again, I surely haven't seen them all 😄).

The Shift key is associated with selection. In combination with Left-Click this also leads to interference with the browser's text selection:

2024-01-05.13.41.Pressing.Shift.mp4

Finally, I am not sure, whether there is an actual use case for a collapse children action, tbh. I tried to find some resources on this, but no luck (If you have anything, please share :)).

What would be nice to have imho, would be a Collapse all action. Some good reasoning on this can be found in this UX StackExchange answer:

https://ux.stackexchange.com/a/102634

As a user I'd like to:
[...]

• Hide the details of all items. This is because I've expanded myself quite a few items and now less items fit in the view [...]

We can't do Expand all, but Collapse all we could do similarly to how VSCode does it:

So, a seperate button on root level of the tree (or above it), that on click will collapse the entire tree down to Root node + First Level. I imagine that would be of great use for editors.

Be that as it may 😅, I suggest, you remove the toggle-related code from this PR, so this functionality can receive more thorough treatment separately.

[grebaldi]

⬆️ This is breaking. Plugins may rely on the signature of this action creator. Adding additional parameters will break any call-site that isn't aware of the change.

To differentiate between the two scenarios ("Toggle collapsed-state of a node" and "Collapse all children of a node"), it'd be better to have an entirely separate redux action (like collapseChildren). The handleToggle method in packages/neos-ui/src/Containers/LeftSideBar/NodeTree/Node/index.js can take care of calling one or the other depending on the given DOM event.

Besides that, as I wrote above already, I don't think that shift+leftclick is an appropriate way to capture the user's intent with regards to this action.

[Devclaim]

Hey, thank you for your elaborate review and insights. I agree, I will work on the collapsing buttons in a different PR and remove the changes from this PR when I find time for that. I was thinking of both a collapse all button (Visual Studio Code) and the shift+leftclick which I am used to from the Godot Editor.

[grebaldi]

Thank you for the reference to Godot :) That is indeed a very good example. I didn't know of that behavior.

[Devclaim]

I have removed the changes to the Toggle action and will create a new PR with the hints from @grebaldi . Thanks again

[grebaldi]

Excellent! Thanks a lot @Devclaim 👍

Looking forward your next PR :) Keep up the good work!

[mhsdesign]

As we currently have only style change i would also propose to disable the selection via user-select: none;

(also note to us, we should squash merge)

[markusguenther]

Thank you very much for your first and awesome contribution.
Guess, the change request from @mhsdesign can be done in a separate PR.

[Alvadda]

As we currently have only style change i would also propose to disable the selection via user-select: none;

(also note to us, we should squash merge)

This is fixed in the seperate PR: 3747