Skip to content

feat(breadcrumbs-flow): task 6 — Mode.ROUTER guard on child management #9485

Open
web-padawan wants to merge 1 commit into
mainfrom
feat/breadcrumbs-task6
Open

feat(breadcrumbs-flow): task 6 — Mode.ROUTER guard on child management #9485
web-padawan wants to merge 1 commit into
mainfrom
feat/breadcrumbs-task6

Conversation

@web-padawan

Copy link
Copy Markdown
Member

Description

Fixes #9482

Depends on #9483

Task 6 of the Breadcrumbs SDD - based on tasks reorder in vaadin/web-components#11912.

Added overrides to disallow manual children modifications when using Mode.ROUTER.

Type of change

  • Feature

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@web-padawan web-padawan force-pushed the feat/breadcrumbs-task6 branch from 3063ad7 to 1dfcb57 Compare June 11, 2026 09:01
@web-padawan web-padawan marked this pull request as ready for review June 11, 2026 09:01
@sonarqubecloud

Copy link
Copy Markdown

@web-padawan web-padawan requested a review from sissbruecker June 11, 2026 09:13
}

@Override
public <V extends @Nullable Object, S extends Signal<V>> void bindChildren(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Once a binding is active you can not go back to having children automatically controlled by the component. So in some way, changing the mode must fail once this has been called. There might be some internals that could be checked whether a child binding is active, rather than adding another state variable.


private Mode mode;

private boolean routerUpdateInProgress;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The name is somewhat misleading because it's also used when clearing children upon switching the mode.

}

@Test
void routerMode_add_throws() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we combine the test cases into one as is done above? Seems a bit excessive at the moment.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does this test mode-specific behavior and should thus maybe be in BreadcrumbsModeTest?

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.

[breadcrumbs] Task 6: Breadcrumbs — Mode.ROUTER guard on child management

2 participants