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

Enable nested navigation templating. #240

Merged
merged 1 commit into from
Dec 12, 2024

Conversation

Hyperkid123
Copy link
Contributor

Jira: https://issues.redhat.com/browse/RHCLOUD-36560

The Jira has links to docs and detailed explanations.

TLDR

A small number of UI modules are injecting navigation items into navigation segments that are not owned by the UI module itself. We need a templating mechanism that allows navigation segments to be injected across different modules.

Trying to do that with some paths/ordering attributes will result in many conflicts and complexity. Allowing explicit templating should make the code stable and configuration easier to understand for developers.

@Hyperkid123 Hyperkid123 requested review from florkbr and a team December 10, 2024 09:02
// currently max known depth is 2, so 10 should be more than enough for ever
// event the depth 2 is challenging when it comes to UX, this should prevent infinite reference loops
if depth > 10 {
return parsedNavItems, fmt.Errorf("maximum navigation depth reached")
Copy link
Contributor

Choose a reason for hiding this comment

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

@Hyperkid123 I think this limitation seems reasonable - though we should write this in our migration guide.

Past that - can we add a test that exercises that this limitation/error is correctly exercised? Otherwise LGTM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yeah. I'll add the test.

duration = time.Second * 10
interval = time.Millisecond * 250
)
ginkgo.It("Should stop navigation nesting if the limit is exceeded", func() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Perfect! Thanks @Hyperkid123

@Hyperkid123 Hyperkid123 merged commit da51728 into RedHatInsights:main Dec 12, 2024
9 checks passed
@Hyperkid123 Hyperkid123 deleted the nested-navigation branch December 12, 2024 08:35
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