-
Notifications
You must be signed in to change notification settings - Fork 30
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
docs: Add UNSAFE documentation to mobile Button and Text #2392
Conversation
Deploying atlantis with
|
Latest commit: |
794c005
|
Status: | ✅ Deploy successful! |
Preview URL: | https://8e8b3efe.atlantis.pages.dev |
Branch Preview URL: | https://taylor-docs-for-unsafe.atlantis.pages.dev |
Deploying atlantis with
|
Latest commit: |
171cb51
|
Status: | ✅ Deploy successful! |
Preview URL: | https://ae6c297f.atlantis.pages.dev |
Branch Preview URL: | https://taylor-docs-for-unsafe.atlantis.pages.dev |
Deploying atlantis with
|
Latest commit: |
c975ab4
|
Status: | ✅ Deploy successful! |
Preview URL: | https://d54410d5.atlantis.pages.dev |
Branch Preview URL: | https://taylor-docs-for-unsafe.atlantis.pages.dev |
Deploying atlantis with
|
Latest commit: |
d463e1c
|
Status: | ✅ Deploy successful! |
Preview URL: | https://529b74e7.atlantis.pages.dev |
Branch Preview URL: | https://taylor-docs-for-unsafe.atlantis.pages.dev |
Deploying atlantis with
|
Latest commit: |
f57feaa
|
Status: | ✅ Deploy successful! |
Preview URL: | https://b291b67b.atlantis.pages.dev |
Branch Preview URL: | https://taylor-docs-for-unsafe.atlantis.pages.dev |
@@ -199,17 +199,3 @@ provided. | |||
<Canvas> | |||
<Text size="small">What is this, a font for 🐜s?</Text> | |||
</Canvas> | |||
|
|||
## Platform Considerations (Mobile) |
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.
Now in TextNotes.mdx
@@ -81,7 +81,7 @@ interface CommonButtonProps { | |||
/** | |||
* **Use at your own risk:** Custom style for specific elements. This should only be used as a | |||
* **last resort**. Using this may result in unexpected side effects. | |||
* More information [here](https://atlantis.getjobber.com/storybook/?path=/docs/guides-customizing-components--docs#unsafe_-props). | |||
* More information in the Customizing components Guide. |
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.
decided to do this instead of:
- Linking back to Storybook
- Links don't show up in the prop table on new docs anyway
@@ -81,7 +81,7 @@ interface CommonButtonProps { | |||
/** | |||
* **Use at your own risk:** Custom style for specific elements. This should only be used as a | |||
* **last resort**. Using this may result in unexpected side effects. | |||
* More information [here](https://atlantis.getjobber.com/storybook/?path=/docs/guides-customizing-components--docs#unsafe_-props). | |||
* More information in the Customizing components Guide. |
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 think it would still be valuable to add the link in case the doc is viewed in Storybook.
* More information in the Customizing components Guide. | |
* More information in the [Customizing components Guide](https://atlantis.getjobber.com/guides/customizing-components). |
However, a more general question, should I be seeing UNSAFE_
prop docs on component pages on the new site? Because I don't see them for Button
or Text
on the new docs site.
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.
Should it be visible only in "Mobile" tab?
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.
Yup - since UNSAFE
is only in the Text
and Button
mobile components right now, it'll only be visible in the mobile tab
Co-authored-by: Nazarii L <[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.
A small suggestion to still include the links (in case the docs are viewed in Storybook) as well as a typo fix suggestion.
@@ -48,7 +48,7 @@ export interface PopoverProps { | |||
/** | |||
* **Use at your own risk:** Custom style for specific elements. This should only be used as a | |||
* **last resort**. Using this may result in unexpected side effects. | |||
* More information [here](https://atlantis.getjobber.com/storybook/?path=/docs/guides-customizing-components--docs#unsafe_-props). | |||
* More information in the Customizing components Guide. |
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.
Perhaps, a link here and on line 40?
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.
Done here f1c32aa
@@ -37,7 +37,7 @@ export interface PopoverProps { | |||
/** | |||
* **Use at your own risk:** Custom class names for specific elements. This should only be used as a | |||
* **last resort**. Using this may result in unexpected side effects. | |||
* More information [here](https://atlantis.getjobber.com/storybook/?path=/docs/guides-customizing-components--docs#unsafe_-props). | |||
* More information in the Customizing components Guide. |
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 line =).
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.
Done here 794c005
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 good!
Motivations
Recently we added
UNSAFE_
props to mobileText
&Button
.UNSAFE_
was going to be used by a team and was somewhat time sensitive, so I committed to updating docs as a fast follow, to get the feature out the door.Changes
Added
Text
Implement tab withUNSAFE_
infoUNSAFE_
info to the pre-existingButton
Implement tabTesting
Make sure you see
UNSAFE_
info in the Implement tab forButton
&Text
.Changes can be
tested via Pre-release
In Atlantis we use Github's built in pull request reviews.