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

[Navigation] Load navigation child items on demand #2526

Open
ky940819 opened this issue Jun 11, 2023 · 0 comments · May be fixed by #2527
Open

[Navigation] Load navigation child items on demand #2526

ky940819 opened this issue Jun 11, 2023 · 0 comments · May be fixed by #2527

Comments

@ky940819
Copy link
Contributor

Feature Request

Is your feature request related to a problem? Please describe.
The Navigation components traverse the entire content tree (within the bounds of the max depth) immediately when the first level of the navigation items are requested. This might be an extremely large structure which may take a non-trivial amount of time to evaluate.

Normally, in the context of how this information is used by the front-end Navigation components, this isn't a big deal because all of those list items are going to be displayed anyway. However, if a user wants to add additional logic in order to prune or modify the list, and they do so by using the sling model delegation pattern, there is no way for them to escape the cost of constructing the entire navigation tree, even if they don't use it all.

Describe the solution you'd like
In the Navigation models, when constructing Navigation Items, the items are a List, requiring that all the children are previously known and constructed. I propose to change this to a Supplier<NavigationItem>, so that how to get the child items is known, but they are not evaluated until/unless required.

This would not effect the Navigation interface, and so would be an invisible change to consumers of Navigation/NavigationItems.
This is just a minor optimization to the backend that would have a trivial impact in normal use, but potentially a profound performance impact in very niche use cases.

Are there alternatives?
Another related feature that might help would be if there was a Sling Model that would supply the Filter<Page>.
Such a model would allow clients to implement just the filtering logic for customizing which NavigationItems are included, without having to rewrite the entire Navigation model. In the absence of such a model, the default new PageFilter() would be used instead (fallback to current functionality).

Documentation
This wouldn't really require any change to the documentation, as it is just a minor back-end optimization.

ky940819 added a commit to ky940819/aem-core-wcm-components that referenced this issue Jun 11, 2023
Updates `NavigationItemImpl` constructor to accept a `Supplier` which
supplies a list of children instead of passing the list its self in
the constructor.

This allows the work of constructing a navigation sub-tree to be
deferred and only completed if the `NavigationItem#getChildren()`
method is called.

----
refs adobe#2526
ky940819 added a commit to ky940819/aem-core-wcm-components that referenced this issue Jun 11, 2023
Updates the V1 and V2 `NavigationImpl` models to pass a `Supplier` to
their respective `NavigationItemImpl` constructors.

Removes `Navigation#getExportedType()` because it is exactly the
same as the super method.

----
refs adobe#2526
ky940819 added a commit to ky940819/aem-core-wcm-components that referenced this issue Jun 11, 2023
Updates the breadcrumb model implementations by removing the `children` field
from `BreadcrumbItemImpl` constructor because it is always set to empty list (i.e.
breadcrumb items never have children).

The `BreadcrumbItemImpl` will now pass a `Supplier` which
supplies an empty list to the super class `NavigationItemImpl`.

Removes `BreadcrumbImpl#getExportedType()` because it is exactly the same
as the super method.

----
refs adobe#2526
ky940819 added a commit to ky940819/aem-core-wcm-components that referenced this issue Jun 11, 2023
…tructors

Updates the V1 and V2 `LanguageNavigationImpl` models to pass
a `Supplier` to their respective `LanguageNavigationItemImpl`
constructors

Fixes a possible NPE in `LanguageNavigationImpl` if
navigation root is not set in either properties or
policy.

Removes `LanguageNavigationImpl#getExportedType()` because it
is exactly the same as the method from the super class.

Update how titles are determined for language navigation items to
reuse the logic used for all other navigation items.

----
refs adobe#2526
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 a pull request may close this issue.

1 participant