Document triggering navigation#4756
Conversation
✅ Deploy Preview for astro-docs-2 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Excluding PDFs and images from transitions is no longer a good example, since now all non-html files are reloaded anyway as well as all html-files that do not have <ViewTransitions /> in their head.
The way it is worded now, it might give the impression that people have to opt out.
We should also clearly state that both pages of the navigation must have <ViewTransitions /> in their <head> to prevent reload. (actually any element with a name=astro-view-transitions-enabled somewhere in the document)
The use case for astro-data-reload would be a navigation to another html page, to which you would normally want transitions (why else would the target page have <ViewTransitions />?), but on this very link between those two pages there should be a full reload. Now I'm starting to run out of imagination. Are there still any other good examples of data-astro-reload?
In the above cases data-astro-reload might be a performance optimizer: The checks for media-type and astro-view-transitions-enabled can only be performed after the target document has been retrieved. If a reload is forced, it must be loaded again. In contrast, data-astro-reload is checked first when a link is clicked, and there is no double loading, which can be important if it takes a long time to generate and load the image that can not be cached.
|
@martrapp Ah yes, that example was added before the media type check was added, so it was the right way to do it at the time. I would probably still use the tag to prevent an unnecessary fetch + redirect, but I agree that there can be a better example. I'll update this. |
sarah11918
left a comment
There was a problem hiding this comment.
OK! I apologize that a Github review is a terrible way to do this. I've made some structural change suggestions, and pointed out a couple of things that will need some clarification and checking.
I really wished this were nicer, but that one stupid deleted line over which I can't edit meant I had to leave several comments around it.
See what you think about incorporating some of my suggestions! And of course, make them accurate. 😅 @matthewp @martrapp
| The `navigate` method takes these arguments: | ||
|
|
||
| - `href` - The path to the page you wish to navigate. The href must be to within the same origin. Any external links will trigger redirection. |
There was a problem hiding this comment.
| The `navigate` method takes these arguments: | |
| - `href` - The path to the page you wish to navigate. The href must be to within the same origin. Any external links will trigger redirection. | |
| The `navigate` method takes one argument: | |
| - `href` - The new page to navigate to. This must be a relative URL to a page within your own domain. Any external links will trigger redirection. |
I can see that more options are planned, but if there's only one, we should really say one. 😄
And, the example above doesn't make it clear what the format should be because the content of the link is actually in a selected value. If someone were writing a path directly, would it be a relative URL like a normal link? (e.g. /about/) I think more explanation is necessary for this.
There was a problem hiding this comment.
The href does not have to be relative. It can be absolute, it can even be complete with protocol, host & post.
And while it may be too complicated to explain here, I suppose it can even have a different origin without triggering a reload if the target page has the correct CORS headers.
Suggestion:
The navigate method takes one argument:
href- The new page to navigate to.
If an error occurs while loading the target page, the navigate function falls silently back to a normal browser load with no view transitions. Typical errors are:
- The document cannot be loaded due to an invalid
href - The document is not an HTML document
- The target page does not define
<ViewTransitions/> - The target page and the current page do not have the same origin (protocol, host, port) and there are no other measures to support CORS (Cross-Origin Resource Sharing).
There was a problem hiding this comment.
We are going to have other options, so I'll go ahead and add them here.
There was a problem hiding this comment.
These have been added now, and the wording on href updated.
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
Co-authored-by: Yan Thomas <61414485+Yan-Thomas@users.noreply.github.com>
sarah11918
left a comment
There was a problem hiding this comment.
Mini review, still working on the second part of the document, which seeing it now makes me want to make some structural changes... so publishing these to get them out of the way!
|
Left lots of little comments above as I made changes, and this new section can be revisited once we have these options all on a reference page, too. For now, I think what we have balances needing to surface the new options released while providing use-case based sections and examples. |
Co-authored-by: Martin Trapp <94928215+martrapp@users.noreply.github.com>
What kind of changes does this PR include?
Description
Related Issue / Implementation PR
For Astro version:
3.2. See Astro PR #.