Skip to content

docs: document that navigating to the shown view does nothing - #25629

Merged
mcollovati merged 2 commits into
mainfrom
issues/25580-make_refreshCurrentRoute_more_visible
Sep 11, 2026
Merged

docs: document that navigating to the shown view does nothing#25629
mcollovati merged 2 commits into
mainfrom
issues/25580-make_refreshCurrentRoute_more_visible

Conversation

@mcollovati

Copy link
Copy Markdown
Collaborator

Part of #25580

DX/docs · flow-server · everyone who navigates to the view that is
already shown

Navigating to the view that is already shown does nothing, and the
javadoc promised the opposite: it said the return value tells you
whether the navigation happened, when it never does. Anyone who wanted
the view rebuilt had no way to find the method that rebuilds it. Both
are documented now; behaviour is unchanged.

Risks:

  • ✅ Javadoc only, plus one test that pins the documented behaviour: no
    behaviour change, no API change, nothing deprecated.

Context. The skip prevents navigation loops, because a BeforeEnter
reroute and the client history sync go through the same call, so the
behaviour stays as it is and only the javadoc was wrong.
refreshCurrentRoute(boolean) already does the rebuild that was asked
for.

  • Documented on all nine UI.navigate overloads that navigating to the
    location that is already shown does nothing: the view is not
    re-instantiated, no navigation lifecycle event fires, and
    refreshCurrentRoute(boolean) is the way to rebuild it.
  • Corrected @return on the seven overloads that return an Optional.
    It said "the view instance, if navigation actually happened"; the
    Optional holds the view that was already shown when the navigation
    was skipped, and is empty when a reroute or a forward went elsewhere.
  • Added the reciprocal cross-reference on refreshCurrentRoute(boolean),
    which now says it is the way to rebuild the view that is currently
    shown and links back to navigate.
  • Added UITest coverage: navigating twice to the same target returns
    the same instance, and refreshCurrentRoute(false) replaces it.

Both UI.navigate and Router.navigate skip a navigation to the location
that is already shown, so the view is not re-instantiated and no
navigation lifecycle event fires. The javadoc said the opposite: every
navigate(Class, ...) overload documented its return value as "the view
instance, if navigation actually happened", while the Optional holds
the view that was already shown. refreshCurrentRoute(boolean), the
method that does rebuild it, was not mentioned anywhere near navigate.

The test pins both halves of the documented contract: navigating twice
returns the same instance, and refreshCurrentRoute replaces it.

Part of #25580
@mcollovati

Copy link
Copy Markdown
Collaborator Author

Type of change

  • Documentation

Javadoc only, so there is nothing to exercise by hand: the review is
reading the wording. mvn javadoc:javadoc -pl flow-server builds with
no reference errors, and UITest passes (77 tests, 1 pre-existing
@Disabled).

Note

This covers option 1 of item 1 in #25580 only. Whether navigate
should return Optional.empty() when it skipped the navigation, and
whether it should gain a force-rebuild overload, were left out as a
separate API ticket, as agreed in the issue. Item 4 of the same issue,
the BeanValidationBinder note about where a constraint message comes
from, is still open and not part of this PR.

@sonarqubecloud

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown
Contributor

Test Results

 1 440 files  ±0   1 524 suites  ±0   1h 41m 25s ⏱️ -54s
12 046 tests +1  11 978 ✅ +1  68 💤 ±0  0 ❌ ±0 
12 364 runs  +1  12 296 ✅ +1  68 💤 ±0  0 ❌ ±0 

Results for commit 63af331. ± Comparison against base commit 3080cc0.

@mcollovati
mcollovati added this pull request to the merge queue Sep 11, 2026
Merged via the queue into main with commit a9187b4 Sep 11, 2026
44 checks passed
@mcollovati
mcollovati deleted the issues/25580-make_refreshCurrentRoute_more_visible branch September 11, 2026 06:37
vaadin-bot added a commit that referenced this pull request Sep 11, 2026
… (CP: 25.3) (#25656)

This PR cherry-picks changes from the original PR #25629 to branch 25.3.
---
#### Original PR description
> Part of #25580
> 
> **DX/docs** · flow-server · everyone who navigates to the view that is
> already shown
> 
> Navigating to the view that is already shown does nothing, and the
> javadoc promised the opposite: it said the return value tells you
> whether the navigation happened, when it never does. Anyone who wanted
> the view rebuilt had no way to find the method that rebuilds it. Both
> are documented now; behaviour is unchanged.
> 
> **Risks:**
> - ✅ Javadoc only, plus one test that pins the documented behaviour: no
>   behaviour change, no API change, nothing deprecated.
> 
> **Context.** The skip prevents navigation loops, because a
`BeforeEnter`
> reroute and the client history sync go through the same call, so the
> behaviour stays as it is and only the javadoc was wrong.
> `refreshCurrentRoute(boolean)` already does the rebuild that was asked
> for.
> 
> - Documented on all nine `UI.navigate` overloads that navigating to
the
>   location that is already shown does nothing: the view is not
>   re-instantiated, no navigation lifecycle event fires, and
>   `refreshCurrentRoute(boolean)` is the way to rebuild it.
> - Corrected `@return` on the seven overloads that return an
`Optional`.
>   It said "the view instance, if navigation actually happened"; the
>   `Optional` holds the view that was already shown when the navigation
> was skipped, and is empty when a reroute or a forward went elsewhere.
> - Added the reciprocal cross-reference on
`refreshCurrentRoute(boolean)`,
>   which now says it is the way to rebuild the view that is currently
>   shown and links back to `navigate`.
> - Added `UITest` coverage: navigating twice to the same target returns
>   the same instance, and `refreshCurrentRoute(false)` replaces it.
>

Co-authored-by: Marco Collovati <marco@vaadin.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants