Skip to content

Conversation

linawolf
Copy link
Contributor

@linawolf linawolf commented Jul 5, 2023

To avoid circular relations we need to mark the root document.

depends on #455, which depends on #443

@jaapio
Copy link
Member

jaapio commented Jul 7, 2023

@linawolf please rebase this one, so we can continue with a review :-)

@linawolf linawolf force-pushed the menu-4 branch 2 times, most recently from 1febe75 to 49a4601 Compare July 9, 2023 17:04
@linawolf
Copy link
Contributor Author

linawolf commented Jul 9, 2023

@jaapio rebased, please review

Copy link
Member

@jaapio jaapio left a comment

Choose a reason for hiding this comment

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

I think this is good for now, some minor things that need to be addressed before we can merge this.


But as I said on Slack. I think we should have a closer look at what we want to store in the nodes. It feels like they are getting larger and larger every feature we add. But most of the new meta like information that is now stored in the documentnode should not be directly used by the renderer. Which means for me that it should not be in the nodes. It is more like our internal administration that nobody should ever look at.

During rendering the renderer should basically just look at nodes and child nodes. All properties exposed by the nodes should add value to the rendering. And I think the DocumentEntryNode doesn't do that. We want people to use the menu nodes an not trying to create there own menu tree in the templates to render a menu.

I think we can address this later.

linawolf added 4 commits July 14, 2023 14:05
This reintroduces the contents directive which was destroyed by switching to transformers
To avoid circular relations we need to mark the root document.
@linawolf linawolf requested a review from jaapio July 14, 2023 14:14
@linawolf
Copy link
Contributor Author

@jaapio please have another look

@linawolf
Copy link
Contributor Author

@jaapio I applied all suggested changes, please have another look

@jaapio jaapio merged commit 36ae6db into main Jul 14, 2023
@jaapio jaapio deleted the menu-4 branch July 14, 2023 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants