Skip to content
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: Updated Status docs to include design guidelines #1940

Merged
merged 4 commits into from Feb 17, 2022
Merged

Conversation

ponori
Copy link
Contributor

@ponori ponori commented Feb 15, 2022

Summary

What changed?

Added Best Practices and Related section and updated Accessibility section of Status docs.

@ponori ponori added the patch release Patch release label Feb 15, 2022
@netlify
Copy link

netlify bot commented Feb 15, 2022

✔️ Deploy Preview for gestalt ready!

🔨 Explore the source changes: f154783

🔍 Inspect the deploy log: https://app.netlify.com/sites/gestalt/deploys/620d7e183db60e0009f4d736

😎 Browse the preview: https://deploy-preview-1940--gestalt.netlify.app

@ponori ponori marked this pull request as ready for review February 15, 2022 21:16
@ponori ponori requested a review from a team as a code owner February 15, 2022 21:16
<MainSection.Card
cardSize="md"
type="do"
description="Place Status close to the status subject in question to provide context and reference. It can be placed as an inline element or paired side by side as needed. "
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kinda wordy…instead of "to the status subject in question", how about "to its subject"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

<MainSection.Card
cardSize="md"
type="do"
description="Use a title when the status it represents is unique, specific and critical for the user to know."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

props should be denoted with backticks and don't need an article, so

Use title when the status it represents…

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was some initial feedback that utilizing prop names may not necessarily be as clear to designers, so we decided to pull is back to something less technical.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

<MainSection.Card
cardSize="md"
type="don't"
description="Use Status' sub text to display extraneous messaging."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto here: subtext refers to the prop, so needs backticks and should match the prop name exactly

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment for this one. I think there's solid arguments on both sides here and I don't know if I have an informed opinion of how much friction this would cause designers.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

@ponori ponori merged commit 6f11cb0 into master Feb 17, 2022
@ponori ponori deleted the pjo-status-docs branch February 17, 2022 02:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch release Patch release
Projects
None yet
2 participants