-
Notifications
You must be signed in to change notification settings - Fork 35
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
Add docs wc #1057
Add docs wc #1057
Conversation
63df11f
to
da0d5f1
Compare
288c4e2
to
3dcd15b
Compare
This comment has been minimized.
This comment has been minimized.
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 is a lot, thanks Fio! A couple comments and questions
packages/ui-extensions/docs/surfaces/admin/staticPages/configuration.doc.ts
Outdated
Show resolved
Hide resolved
55dbf59
to
6875593
Compare
packages/ui-extensions/src/surfaces/admin/components/shared/index.ts
Outdated
Show resolved
Hide resolved
packages/ui-extensions-react/src/surfaces/admin/components/Form/examples/basic-form.example.tsx
Outdated
Show resolved
Hide resolved
packages/ui-extensions/src/surfaces/admin/components/Form/examples/basic-form.example.ts
Show resolved
Hide resolved
packages/ui-extensions/docs/surfaces/admin/staticPages/configuration.doc.ts
Outdated
Show resolved
Hide resolved
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 gone through all the content but mostly just the home page and left a few suggestions and rewrites;
Things to avoid: using words like "We".
Things to do before shipping:
- Grep for the word shopify; and make sure it's capitalized when in body content.
- Look for instances where it makes sense to markup content with inline code elements for easier reading.
anchorLink: 'customProtocols', | ||
accordionContent: [ | ||
{ | ||
title: 'shopify:admin', |
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.
@johndcollett is it possible to wrap these in a <code>
element so they don't get capitalized as titles since they're reference to actual code strings?
Other option would be to think about the titles more like titles;
Admin protocol
, App protocol
, Extension protocol
, and Relative URLs
and in the body use a code element in the description so it's more obvious what the protocol is.
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 would take a code change to be able to do something like that. Right now there are just expecting strings and capitalize the first letter. We can remove that functionality but it might lead to more titles with lowercase letter happening.
packages/ui-extensions/docs/surfaces/admin/staticPages/admin-extensions.doc.ts
Outdated
Show resolved
Hide resolved
packages/ui-extensions/docs/surfaces/admin/staticPages/admin-extensions.doc.ts
Outdated
Show resolved
Hide resolved
packages/ui-extensions/docs/surfaces/admin/staticPages/admin-extensions.doc.ts
Outdated
Show resolved
Hide resolved
packages/ui-extensions/docs/surfaces/admin/staticPages/admin-extensions.doc.ts
Outdated
Show resolved
Hide resolved
packages/ui-extensions/docs/surfaces/admin/staticPages/configuration.doc.ts
Outdated
Show resolved
Hide resolved
sectionCard: [ | ||
{ | ||
name: 'Getting Started', | ||
subtitle: 'Get started building your first Admin Extension.', |
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.
The subtitle copy is meant to be more of a description of the type of content you're linking to
![Screenshot 2023-07-13 at 12 14 25 PM](https://private-user-images.githubusercontent.com/1638215/253341109-87c23e7e-9c44-4b0a-818a-8df78c6c078f.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MjE0NDg2NjIsIm5iZiI6MTcyMTQ0ODM2MiwicGF0aCI6Ii8xNjM4MjE1LzI1MzM0MTEwOS04N2MyM2U3ZS05YzQ0LTRiMGEtODE4YS04ZGY3OGM2YzA3OGYucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI0MDcyMCUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNDA3MjBUMDQwNjAyWiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9M2FiNWVkZDJkYWMwM2I4YjM1NWM3NTg4NGQ4MTU5OGUxOTJmOWFlOGIwMTlmYzYxZmFiYmEyYWRmZmY4YTFmMSZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QmYWN0b3JfaWQ9MCZrZXlfaWQ9MCZyZXBvX2lkPTAifQ.UoMzYk1V6kU63l0PwEDUdJ0edRKAZ8R2SfqskLq7hyM)
See Hydrogen for a great example of how to structure these;
https://shopify.dev/docs/api/hydrogen-react
![Screenshot 2023-07-13 at 12 17 44 PM](https://private-user-images.githubusercontent.com/1638215/253341932-87b830f8-6204-4bac-a9b2-42feb55a55ce.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MjE0NDg2NjIsIm5iZiI6MTcyMTQ0ODM2MiwicGF0aCI6Ii8xNjM4MjE1LzI1MzM0MTkzMi04N2I4MzBmOC02MjA0LTRiYWMtYTliMi00MmZlYjU1YTU1Y2UucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI0MDcyMCUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNDA3MjBUMDQwNjAyWiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9MTlhYTk1NmViMGUxYmU1ODRhNzQwMjJjMDUwNTE0ZmU4NGIxNjlmZDYxMDhlYWI3Njg1MzdjMjE2MjdkNzEwYyZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QmYWN0b3JfaWQ9MCZrZXlfaWQ9MCZyZXBvX2lkPTAifQ.xeLRgDRegWbuLeO8RfkLdkHAkp-8FNkkniy3jj6GsdU)
![Screenshot 2023-07-13 at 12 17 40 PM](https://private-user-images.githubusercontent.com/1638215/253341978-50cb2739-213a-43de-b302-cd8fbfc1dfb9.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MjE0NDg2NjIsIm5iZiI6MTcyMTQ0ODM2MiwicGF0aCI6Ii8xNjM4MjE1LzI1MzM0MTk3OC01MGNiMjczOS0yMTNhLTQzZGUtYjMwMi1jZDhmYmZjMWRmYjkucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI0MDcyMCUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNDA3MjBUMDQwNjAyWiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9NjNhNjJhMWI0OWM3ZWQ5MDJmZTFmNDAxYTNjMDljMGU2OTdhYTdhODUxOWFkZjY2MTRiYzk4ZjA1NDQxNGFjYyZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QmYWN0b3JfaWQ9MCZrZXlfaWQ9MCZyZXBvX2lkPTAifQ.--2PY9y53IXBhukqTGcTFnOsj387LCp1Idg_nVdBocM)
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.
We also need a card to link to the Figma UI Kit, like what Checkout did
https://shopify.dev/docs/api/checkout-ui-extensions#ui-components
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.
Updated these. @lizkhoo-shop do we have a public link for the figma ready?
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.
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.
No link for the Figma kit yet. We were planning to publish that on July 25, but I think it's okay if there's nothing here for now. You could put '(coming soon)' next to the title.
A few copy notes:
- We don't use '.' period punctuation at the end of the big titles
- "Admin Extension" should be lower case "admin extension"
- Direct API access card - 'SHopify' should be 'Shopify'
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.
Updated
packages/ui-extensions/docs/surfaces/admin/staticPages/admin-extensions.doc.ts
Outdated
Show resolved
Hide resolved
packages/ui-extensions/docs/surfaces/admin/staticPages/admin-extensions.doc.ts
Outdated
Show resolved
Hide resolved
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.
A few more spelling mistakes.
4f71231
to
216de8c
Compare
Co-authored-by: Tiffany Tse <tiffanytse@users.noreply.github.com>
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.
Looks great!
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.
Thanks for doing this!
Background
This PR primarily uses the .dev doc generator + our own custom bash script to scaffold the docs.
What's in this PR
*.doc.ts
files for every component in the Admin surface of UI Extensions*.doc.ts
for APIs for admin ui extensionsChecklist