Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FEATURE: Collapse All Button in Content and Page Tree #3756

Merged
merged 8 commits into from Apr 24, 2024

Conversation

Devclaim
Copy link
Contributor

@Devclaim Devclaim commented Mar 16, 2024

What I did

Add a Button to collapse all collapsable Nodes in the Content and Page Tree in the LeftSideBar. Since I didn't like how it looked as an IconButton, it's just text indicating exactly what it does.

Note: Currently Nodes that are closed by default behave differently than other Nodes (Instead of being added to the toggled state in the Redux Store when collapsed, they are added when they are expanded). I wasn't able to make it work without generating new Bugs so I just gave up, but I think this should be looked into at some point.

How I did it

  • Add a selector to either select all document nodes or content nodes that are collapsable (DocumentNodes must have children with subtypes of Document and ContentNodes must have children with SubTypes of Content and contentCollection )

  • Add the Button on the top right of the Tree Container

How to verify it

Open all the Nodes and click "Collapse all" :)

collapseAllPreview

@github-actions github-actions bot added Feature Label to mark the change as feature 8.4 labels Mar 16, 2024
@Sebobo Sebobo self-requested a review March 19, 2024 09:59
Copy link
Member

@Sebobo Sebobo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great addition!
Left some first feedback, and didn't test the feature itself yet in the backend.

packages/neos-ui-redux-store/src/CR/Nodes/selectors.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@grebaldi grebaldi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Devclaim,

excellent solution! 💯

I left some nitpick comments on the code, but I was also able to test your change locally and it works like a charm 👍

I know this one was tough to figure out. Thanks a lot for staying on it!

Thanks for your effort, this is great work!

@Devclaim
Copy link
Contributor Author

Devclaim commented Mar 28, 2024

Thank you for your Feedback, I think I have replied to all your suggestions. I do need some help with the translations though.
see this comment: #3756 (comment)

Copy link
Contributor

@grebaldi grebaldi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Devclaim,

thanks so much for your adjustments! Sorry for the delayed response, I've been busier with 9.0 stuff than I anticipated.

Your integration of the i18nRegistry is absolutely correct 👍 I've added a code suggestion to solve the remaining issue. After that, we're good to go :)

Co-authored-by: Wilhelm Behncke <2522299+grebaldi@users.noreply.github.com>
@Devclaim
Copy link
Contributor Author

Thank you, that fixed the Issue, I will add the remaining translations asap

Copy link
Contributor

@grebaldi grebaldi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @Devclaim! :)

Copy link
Member

@Sebobo Sebobo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and works great!

@Sebobo Sebobo merged commit 95a5583 into neos:8.4 Apr 24, 2024
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.4 Feature Label to mark the change as feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants