fix: fix anchor navigation with react router

[tltv]

Fixed navigation via anchor to Flow views not rendering content. Removed unnecessary if-block from Flow.tsx that stopped rendering the view.

Fixes: #18896

[tltv]

Removed if-block was added originally along test PR. Related integration tests works without this if-block (locally. CI runs tests atm so don't know the result at the time writing this).

@caalador do you remember what was the original purpose of this if-block check?

[github-actions[bot]]

Test Results

1 096 files  ±0  1 096 suites  ±0   1h 22m 50s :stopwatch: +50s 6 978 tests ±0  6 929 :white_check_mark: ±0  49 :zzz: ±0  0 :x: ±0  7 327 runs  ±0  7 266 :white_check_mark: ±0  61 :zzz: ±0  0 :x: ±0 

Results for commit a0ac1946. ± Comparison against base commit e36214bb.

:recycle: This comment has been updated with latest results.

[caalador]

That was when react router always generated 2 roundtrips to the server so we blocked the second navigation round. Check that one navigation in flow only generates a single request that gets all the changes back and not 2 for a navigation. Not that this is about a pure flow application and hilla/react views should not be in the mix.

[caalador]

Also check that the vaadinRouterGlobalClickHandler(event) is called for the link click and which path it takes.

[tltv]

That was when react router always generated 2 roundtrips to the server so we blocked the second navigation round. Check that one navigation in flow only generates a single request that gets all the changes back and not 2 for a navigation. Not that this is about a pure flow application and hilla/react views should not be in the mix.

There are two requests only when navigating via anchor to Flow view. First one leaves just blank page, second one updates it. Is this expected behavior or not? Should it be able to do it with one request instead?

Also check that the vaadinRouterGlobalClickHandler(event) is called for the link click and which path it takes.

I'm not sure what path you mean exactly, but it reads the target navigation path from the correct anchor element there.

[tltv]

First anchor click event is not detected as flow view here. That's why the second request is needed it seems. But could we detect this as a flow view in the first place here?

[tltv]

Sorry to flood with comments @caalador but if I change the if-block in the previous comment's screenshot to this: if(matched?.length > 0 && matched[matched.length - 1].route.path === "/*") then anchor works with just one request.

Can't detect any side effects yet.

[caalador]

So the main issue is that the matcher returns targets / and /* where we expect only /* Why would it suddenly start to return different results...

[tltv]

fs-routing is now always used with hilla. buildRoute() combines server side routes in root "/" route's children array by default which means that matchRoutes function returns also the root path "/".

[caalador]

I guess the fs-routing always setting the / is the reason we can't have Route("") in flow working

[tltv]

My last change breaks navigation menu's automatic highlighting somehow. It worked before when it still though it was updating client side route. Now that it updates for server side route, highlighting is not working.

[tltv]

Fixed highlighting issue. Also fixed wrong "!==" to "===" in code that is supposed to run special cleanup task only for Flow page.

[sonarqubecloud[bot]]

Quality Gate passed

Issues
6 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

[tltv]

Highlighting NavLink in the react menu automatically when navigating via anchor in Flow view to another Flow view is now broken but I think that can be fixed separately. Problem is that now we never call react router's navigation and for some reason react router is not reacting to the location change to refresh the menu.

Afaik usage of "/" path for the main layout is React router's feature. We have also this one related to that https://github.com/vaadin/flow/issues/18966.

[vaadin-bot]

This ticket/PR has been released with Vaadin 24.4.0.alpha16 and is also targeting the upcoming stable 24.4.0 version.