Skip to content
This repository has been archived by the owner on Sep 16, 2021. It is now read-only.

[POC] Provide a faster menu helper #241

Closed
wants to merge 7 commits into from
Closed

[POC] Provide a faster menu helper #241

wants to merge 7 commits into from

Conversation

wouterj
Copy link
Member

@wouterj wouterj commented Jan 9, 2016

The KnpMenu library is a very generic library, providing tools which can be used in many apps. The CMF however is much more specific, implementing a much better backend for trees (PHPCR) and having a specific way to store the current content/route.

This means that a lot of things can be done much more efficient (e.g. retrieving the current item and generating a breadcrumb array). This PR is a proof of concept and a very early sketch of some faster templating helpers.

Partly fixes #19

@wouterj wouterj added the wip/poc label Jan 9, 2016
return $breadcrumbs;
}

return array_merge($breadcrumbs, $this->getBreadcrumbArray($item->getParentObject()));
Copy link
Member Author

Choose a reason for hiding this comment

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

is there a fast way to get the current item and all its parents in one call (query) with PHPCR?

Copy link
Member

Choose a reason for hiding this comment

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

not built-in. but we do this for the route loading: https://github.com/symfony-cmf/Routing/blob/master/Candidates/Candidates.php#L141-L147

if we are PHPCR specific here, i guess its safe to do the same (assume that / is separator for the id). if i remember correctly, $dm->findMany also preserves the order. in the linked example, we want to go from child to parent (routing wants to match the most specific possible) while here you want to invert the order to have parent first.

@wouterj
Copy link
Member Author

wouterj commented Jan 20, 2016

PR should be somewhat ready, implemented content support by reusing the MenuNodeReferrerInterface.

I'll test & benchmark this this weekend. In case it is faster, I would like to merge it and tag CmfMenuBundle 2.0.0-RC1 on Sunday. /cc @dbu @lsmith77 fine by you?

@dbu
Copy link
Member

dbu commented Jan 21, 2016 via email

@wouterj
Copy link
Member Author

wouterj commented Jan 23, 2016

Let's skip this for 2.0, it contains to much bugs (especially not being able to select menu node with content) and I cannot really discover if it's faster or not (blackfire says it's faster, but shows lots of unrelated stuff in the tree).

@wouterj wouterj modified the milestone: 2.1 Jun 18, 2016
@ElectricMaxxx ElectricMaxxx modified the milestones: 2.2, 2.1 Nov 28, 2016
@ElectricMaxxx
Copy link
Member

pls reopen if still interessted.

@ElectricMaxxx ElectricMaxxx deleted the breadcrumb branch May 16, 2018 03:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

prevent KnpMenu from loading the entire menu/route/content tree
3 participants