docs: New route for read random article in documentation#68043
docs: New route for read random article in documentation#68043aparzi wants to merge 1 commit intoangular:mainfrom
Conversation
|
My idea was to navigate directly to an existing route with a basic function in the search dialogue, like Wikipedia random article feature, I suppose you have a different idea with a new route and a page |
Add random route: 'angular.dev/random' for read random article Fixed angular#68038
67c6485 to
d9f55cc
Compare
I wanted to set it up this way for now, and I'd like to see some feedback from the team. Currently, the random route redirects to a random page. |
Tbh I'm excited to see it, as soon as i have a chance, I will have a look, thanks for effort 👌 |
| }, | ||
| { | ||
| path: PAGE_PREFIX.RANDOM, | ||
| loadComponent: () => import('../features/random/random'), |
There was a problem hiding this comment.
Not really sure it’s worth the cost of adding an extra component.
We could just use redirectTo for this: https://angular.dev/guide/routing/redirecting-routes#conditional-redirects
Or maybe browserUrl for this case: https://angular.dev/guide/routing/navigate-to-routes#display-a-different-url-in-the-address-bar
Is there any specific reason to do it this way?
wdyt some like
export const randomRedirectGuard: CanActivateFn = () => {
// logic to build random URL
return router.navigateByUrl(random, {
browserUrl: '/random',
});
};
export const routes = [
{
path: 'random',
children: [],
canActivate: [randomRedirectGuard],
}
]There was a problem hiding this comment.
No, there's no specific technical reason. I just wanted to set it up this way and see the initial feedback. I agree that the component is perhaps excessive.
erkamyaman
left a comment
There was a problem hiding this comment.
there's no UI link (like a nav item or button) that points to /random, it's only accessible by navigating to /random directly in the URL. i guess it was intentional for now.
i tried your code on my machine by navigating to /random directly in the URL, all works well, thank you!
| const allItems = [ | ||
| ...flatNavigationData(SUB_NAVIGATION_DATA.docs), | ||
| ...flatNavigationData(SUB_NAVIGATION_DATA.reference), | ||
| ...flatNavigationData(SUB_NAVIGATION_DATA.tutorials), | ||
| ]; | ||
|
|
||
| const paths = allItems | ||
| .map((item) => item.path) | ||
| .filter((path): path is string => !!path && !path.startsWith('http')); | ||
|
|
||
| if (paths.length > 0) { | ||
| const randomPath = paths[Math.floor(Math.random() * paths.length)]; | ||
| this.router.navigateByUrl(`/${randomPath}`, {replaceUrl: true}); | ||
| } else { | ||
| this.router.navigateByUrl('/', {replaceUrl: true}); | ||
| } | ||
| } |
There was a problem hiding this comment.
this logic does the job well!
just one thing but nit!
The current implementation can redirect the user back to the page they came from.
If I'm on /guide/components/selectors and hit /random, it could randomly pick /guide/components/selectors again and send me right back where I started.
| }, | ||
| { | ||
| path: PAGE_PREFIX.RANDOM, | ||
| loadComponent: () => import('../features/random/random'), |
There was a problem hiding this comment.
i agree on that one with @SkyZeroZx we don't need a new component
Add random route: 'angular.dev/random' for read random article
Fixed #68038
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Issue Number: #68038
What is the new behavior?
Add new route '/randon' in angular documentation for read a random article
Does this PR introduce a breaking change?