-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
fix(dev-toolbar-app-reference): API reference format + new component #12189
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
fix(dev-toolbar-app-reference): API reference format + new component #12189
Conversation
✅ Deploy Preview for astro-docs-2 ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project 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.
This looks amazing @ArmandPhilippot !
I've left comments re: the doubts and visited most (but not all 🫣 ) of the links to confirm your choices. 😅
I'm comfortable with what you've added, and would love others to review as well, especially the things you're not sure of!
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.
Just checked the updated deploy preview and everything looks (visually) good on the page!
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.
LGTM! thanks
See withastro#12189 + rewording
Description (required)
Updates
dev-toolbar-app-reference.mdx
to:astro-dev-toolbar-select
component added in feat(toolbar): allow the user to change the placement astro#10591icons
list (see https://github.com/withastro/astro/blob/02366e9ce38df8e7362817c095ff05ae61dc7b56/packages/astro/src/runtime/client/dev-toolbar/ui-library/icons.ts#L25)Here's the details of the replacements:
Signature
withType
for consistency. I think this is the only page usingSignature:
.Type
/Default
/Since
.init()
return type (can be aPromise
, see https://github.com/withastro/astro/blob/15b55f34cb84ecfb99d2e76918a567a00bbb13f6/packages/astro/src/types/public/toolbar.ts#L53-L57).beforeTogglingOff()
return type (can be aPromise
but can't bevoid
, see https://github.com/withastro/astro/blob/15b55f34cb84ecfb99d2e76918a567a00bbb13f6/packages/astro/src/types/public/toolbar.ts#L58)onInitialized
should beonAppInitialized
(see https://github.com/withastro/astro/blob/02366e9ce38df8e7362817c095ff05ae61dc7b56/packages/astro/src/integrations/hooks.ts#L137)addDevToolbarApp
text because theicon
property is optional while others are required.toggle-style
inastro-dev-toolbar-toggle
(see https://github.com/withastro/astro/blob/02366e9ce38df8e7362817c095ff05ae61dc7b56/packages/astro/src/runtime/client/dev-toolbar/ui-library/toggle.ts#L10)radio-style
to be consistent with other components (see https://github.com/withastro/astro/blob/02366e9ce38df8e7362817c095ff05ae61dc7b56/packages/astro/src/runtime/client/dev-toolbar/ui-library/radio-checkbox.ts#L6)style
property inastro-dev-toolbar-highlight
to usehighlight-style
and to stay consistent with other components (see https://github.com/withastro/astro/blob/02366e9ce38df8e7362817c095ff05ae61dc7b56/packages/astro/src/runtime/client/dev-toolbar/ui-library/highlight.ts#L102)Doubts:
addDevOverlayPlugin()
has been replaced withaddDevToolbarApp()
in4.0.0
. However the signature has changed in4.7.0
. So I'm not entirely sure if we should use4.0.0
or4.7.0
here. See:level
while the property is optional and the default seems to be set elsewhere looking at the implementation. So, should we use a default or not? Note, I've checked anderror
is still the default as described in the text.icon
is optional, should we precise a default? Without icon,?
is displayed so I used that as value (see https://github.com/withastro/astro/blob/02366e9ce38df8e7362817c095ff05ae61dc7b56/packages/astro/src/runtime/client/dev-toolbar/entrypoint.ts#L211).In case anyone want to check the types/components, here's a small app using almost all the APIs: https://stackblitz.com/edit/astro-dev-toolbar-app-example?file=my-toolbar-app%2Fmy-integration.ts,my-toolbar-app%2Fmy-app.ts&on=stackblitz (you can download it and open it in your editor to be able to see the signatures)
Related issues & labels (optional)
add new content
,consistency/formatting