-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Document ViewTransitions API reference #9174
Conversation
✅ Deploy Preview for astro-docs-2 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fixed a few links, feel free to double check that this was the correct resource. I think there are still some broken links.
Other than that (and a typo), my comments are mostly related to the formatting of Type:
and <Since />
. We have updated the api-reference.mdx
file to make the formatting of this information more consistent (see #8943, the format reference is the one used in configuration-reference.mdx
):
<Since />
andType:
should be inside the same paragraph- If a default value exists, a
Default:
line should be added.
So I added some suggestions on this. I didn't double check each function. If you think of a missing Default:
don't hesitate to add it in the same block! 😄
Co-authored-by: Armand Philippot <[email protected]>
Thank you @ArmandPhilippot for that great consistency work! So appreciated! 💪 I think it's time for @martrapp to have a look at what we've all been up to.... 👀 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems I missed two incorrectly formatted Type:
in the previous review. 👀
Co-authored-by: Armand Philippot <[email protected]>
Noting that the link should work after the new View Transitions docs have merged tomorrow! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow! My wholehearted thanks to the Arcane Tower 🧙🏼♂️ and everyone who helped. I've just added a few small suggestions that you may or may not want to consider.
**Type:** `() => Promise<void>` | ||
</p> | ||
|
||
Implementation of the following phase in the navigation (loading the next page). This implementation can be overriden to add extra behavior or to prevent the execution of the next phase. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... or to prevent the execution of the next phase
That is a mystery to me. Do we have any examples of this anywhere? Why would you do that? And why does it cancel the next phase? Calling preventDefault()
for the astro:before-preparation
event, on the other hand, is fine.
If we do not persuade users of this, we do not need to warn them in the following
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't seen an example, nor can I think of any particular use case where you'd want to override this and not call the original at some point in the new function, but technically, you can.
I'm fine with not mentioning it on the docs tho.
Just resolved conflicts/updated branch! With any luck, that means the link check now passes! |
Co-authored-by: Martin Trapp <[email protected]>
Needed to resolve conflicts again because this page just updated! 😂 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much @Fryuni for taking the initiative here! Would not be possible without the outstanding contributions of @ArmandPhilippot and the wise oversight of @martrapp .
Best Docs Team Ever! 🥳
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good work! I have mostly nit and typo suggestions here, after applying them I believe it's good to go!
Co-authored-by: Yan <[email protected]>
Co-authored-by: Yan <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some quick tidying
#9260) * i18n(fr): add Actions in `api-reference.mdx` See #9224 and #9254 * i18n(fr): add View Transitions section in api-reference.mdx See #9174 * i18n(fr): improve Actions handler documentation See #9261 * i18n(fr): update `view-transitions.mdx` See #9224 * i18n(fr): reword sentences around handler to make reading easier * i18n(fr): fix View Transitions capitalization where legit Co-authored-by: Thomas Bonnet <[email protected]> --------- Co-authored-by: Thomas Bonnet <[email protected]> Co-authored-by: Yan <[email protected]>
Description
Add a reference to the APIs and events available when using View Transitions.
Related issues & labels (optional)