Skip to content

Route Redirect fixes #19409

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

Open
wants to merge 2 commits into
base: release/16.0
Choose a base branch
from
Open

Conversation

nielslyngsoe
Copy link
Member

The cases described here fixes an issue that only appeared on an actual server. these can be seen on a local server by running with disabled cache and a network throttling like this:
image

This fixes two things:
A.
Fixes so a deep link does not just redirect to the root of a section, typically section/settings — this is resolved by using pathMatch 'full', a feature of the router slot we have not utilized previously.

B.
Fixes so the initial empty path load does not go to section/settings, just because it loaded slightly before the content section. This is done by introducing a feature that makes redirects await a delay, the delay will be reset everytime a new route appears, making it able to extend the time as long as routes are getting registered within the given delay. And then the delay is calculated based on the browsers assumption on the internet speed. That's a guess but better than none.

@Copilot Copilot AI review requested due to automatic review settings May 24, 2025 10:02
@nielslyngsoe nielslyngsoe requested a review from iOvergaard May 24, 2025 10:02
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enhances route redirects by enforcing full path matching on default routes and introduces a delay mechanism for initial redirects to ensure all routes register before navigation.

  • Add pathMatch: 'full' to empty-path redirects to prevent unintended root fallback.
  • Implement awaitStability with a network-based delay in RouterSlot for initial redirect timing.
  • Extend the redirect model (IRedirectRoute) to include the optional awaitStability flag.

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
created-packages-section-view.element.ts Added pathMatch: 'full' to default redirect.
member-workspace-editor.element.ts Added pathMatch: 'full' to default redirect.
media-workspace-editor.element.ts Added pathMatch: 'full' to default redirect.
media-dashboard.element.ts Added pathMatch: 'full' to default redirect.
document-blueprint-workspace-editor.element.ts Added pathMatch: 'full' to default redirect.
content-editor.element.ts Added pathMatch: 'full' to default redirect.
content-type-design-editor.element.ts Added pathMatch: 'full' to default redirect.
block-workspace-view-edit.element.ts Added pathMatch: 'full' to default redirect.
backoffice-main.element.ts Added pathMatch: 'full' and awaitStability: true to initial redirect.
router-slot.ts Added getRedirectDelay() and delay logic around awaitStability.
model.ts Extended IRedirectRoute with awaitStability?.
example-modal.element.ts Added pathMatch: 'full' to default redirect in modal example.
dashboard.element.ts Added pathMatch: 'full' to default redirect in dashboard example.
Comments suppressed due to low confidence (1)

src/Umbraco.Web.UI.Client/src/packages/core/router/router-slot/router-slot.ts:408

  • Add unit tests for the awaitStability logic to verify that redirects are correctly delayed and canceled when navigation is invalidated.
if (route.awaitStability === true) {

@@ -314,6 +314,24 @@ export class RouterSlot<D = any, P = any> extends HTMLElement implements IRouter
}
}

Copy link
Preview

Copilot AI May 24, 2025

Choose a reason for hiding this comment

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

Consider adding a JSDoc comment above getRedirectDelay() to explain the heuristics and intent of using network-effective types for delay values.

Suggested change
/**
* Determines the delay (in milliseconds) for redirects based on the network's effective type.
*
* This function uses the `navigator.connection.effectiveType` property to assess the network
* conditions and returns a delay value accordingly. The mapping is as follows:
* - 'slow-2g' or '2g': 1200ms
* - '3g': 800ms
* - '4g': 200ms
*
* If the `navigator.connection` API is unavailable or the effective type is not recognized,
* a default delay of 400ms is returned.
*
* Note: The `navigator.connection` API may not be supported in all browsers, and its behavior
* can vary across environments.
*/

Copilot uses AI. Check for mistakes.

Comment on lines +325 to +332
return 1200;
case '3g':
return 800;
case '4g':
return 200;
}
}
return 400;
Copy link
Preview

Copilot AI May 24, 2025

Choose a reason for hiding this comment

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

Extract these numeric delays (1200, 800, 200, 400) into named constants (e.g., REDIRECT_DELAY_2G) to improve readability and ease future adjustments.

Suggested change
return 1200;
case '3g':
return 800;
case '4g':
return 200;
}
}
return 400;
return REDIRECT_DELAY_2G;
case '3g':
return REDIRECT_DELAY_3G;
case '4g':
return REDIRECT_DELAY_4G;
}
}
return REDIRECT_DELAY_DEFAULT;

Copilot uses AI. Check for mistakes.

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.

1 participant