fix(aio): set the pageId to the file-not-found URL if the doc is not available#16085
Conversation
petebacondarwin
added
comp: aio
action: review
|
Oops I forgot to add the test of the fix 😱 |
aio/src/app/app.component.ts
Outdated
There was a problem hiding this comment.
this.currentDocument --> doc (?)
Can doc really be falsy? Looking at the DocumentService, I think not.
Also, doc might not have a url property (but it might actually be a DocumentService bug).
There was a problem hiding this comment.
I agree. I think it was from a dodgy test, which I have fixed
aio/src/app/app.component.ts
Outdated
There was a problem hiding this comment.
Why is the logic here different than before? Was it a bug in the previous implementation (where pageId was set to home only if falsy)?
There was a problem hiding this comment.
Previously we were getting the url from the navigationService, which returns "" for the home page.
Now we are getting the url from the documentService, which returns "index" for the home page.
There was a problem hiding this comment.
I can't comment on the next line, but do we need to set a url there too?
There was a problem hiding this comment.
yes you are right
|
The angular.io preview for 7c1a895455b47769c1b6f1e4cd360548a00a2c19 is available here. |
petebacondarwin
force-pushed
the
aio-not-found-id
branch
from
49c0cd0 to
b7b94a8
Compare
|
@gkalpak PTAL |
|
The angular.io preview for 49c0cd0c7b33a6d94283167575af15906b734173 is available here. |
|
The angular.io preview for b7b94a8c49b7becb2404917ee439635f11414d56 is available here. |
…available Previously, the `AppComponent.pageId` was set via the current URL, rather than the document being displayed. This is only really noticeable when the URL does not match a valid doc and we are actually displaying a 404 page. Now we compute the `pageId` from the URL of the document being viewed, which is returned from the `DocumentService.currentDocument` observable instead.
petebacondarwin
force-pushed
the
aio-not-found-id
branch
from
b7b94a8 to
f3e6812
Compare
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
5 participants
Previously, the
AppComponent.pageIdwas set via the current URL, rather thanthe document being displayed. This is only really noticeable when the URL does not
match a valid doc and we are actually displaying a 404 page.
Now we compute the
pageIdfrom the URL of the document being viewed,which is returned from the
DocumentService.currentDocumentobservable instead.