Coverage improvements #449
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
langermank
approved these changes
Mar 23, 2023
Co-authored-by: Katie Langerman <18661030+langermank@users.noreply.github.com>
emilybrick
commented
Mar 23, 2023
emilybrick
commented
Mar 23, 2023
| @@ -231,6 +231,13 @@ Buttons allow users to initiate an action or command when clicked or tapped. The | |||
|
|
|||
| ## Best practices | |||
|
|
|||
| ### Primary and outline button usage | |||
There was a problem hiding this comment.
Suggested change
| ### Primary and outline button usage | |
| ### Primary button usage |
emilybrick
commented
Mar 23, 2023
| alt="An image showing a button group in medium and small sizes." | ||
| /> | ||
|
|
||
| ### Leading and trailing visuals |
There was a problem hiding this comment.
@mperrotti I don't know if these should be referred to as Leading and trailing visuals? Wdyt
emilybrick
commented
Mar 23, 2023
| @@ -13,8 +13,6 @@ export default ComponentLayout | |||
| import {Box} from '@primer/react' | |||
| import {Caption} from '@primer/gatsby-theme-doctocat' | |||
|
|
|||
| ## Overview | |||
There was a problem hiding this comment.
removed bc this is also what the overarching tab is called, so felt confusing. Usage is a section underneath, so can't dupe that here either
mperrotti
reviewed
Mar 23, 2023
Comment on lines
+74
to
+83
| ### Leading and trailing visuals | ||
|
|
||
| Similarly to buttons, button segments can optionally include an icon and/or a counter. | ||
|
|
||
| <img | ||
| src="https://user-images.githubusercontent.com/586552/226987735-471711f1-5342-4605-903d-889301eace46.png" | ||
| role="presentation" | ||
| width="960" | ||
| alt="An image showing a button group in medium and small sizes." | ||
| /> |
There was a problem hiding this comment.
Non-blocking nitpick: I think we could get rid of this part and just rely on the button component docs.
| @@ -231,6 +231,13 @@ Buttons allow users to initiate an action or command when clicked or tapped. The | |||
|
|
|||
| ## Best practices | |||
|
|
|||
| ### Primary and outline button usage | |||
|
|
|||
| Limit primary button usage. Only use one primary button on the page, whenever possible, to indicate its emphasis relative to other actions. Primary buttons have top priority in visual hierarchy, and using too many of them on a single view dilutes their effectiveness. | |||
|
|
||
| ## Usage | ||
|
|
||
| The popover component is used to deliver context-specific information and functionality. It’s a small dialog to bring attention to specific user interface elements. It can provide additional information, options, or actions related to a specific element or task. Popovers can be helpful for flows that require light onboarding or instruction. |
There was a problem hiding this comment.
Suggested change
| The popover component is used to deliver context-specific information and functionality. It’s a small dialog to bring attention to specific user interface elements. It can provide additional information, options, or actions related to a specific element or task. Popovers can be helpful for flows that require light onboarding or instruction. | |
| The popover component is used to deliver context-specific information and functionality. It’s a small non-modal dialog to bring attention to specific user interface elements. It can provide additional information, options, or actions related to a specific element or task. Popovers can be helpful for flows that require light onboarding or instruction. |
Comment on lines
+30
to
+32
| Use sparingly to avoid cognitive overload. Though they can be used for a variety of things, they should be used sparingly to avoid cognitive overload. It's important to consider the context in which the popover appears. Are there other popovers on the page? Does it appear on page load, or require the user to open the popover? | ||
|
|
||
| Unlike other messaging components, popovers should never include critical information (such as errors) and should always include a dismiss action. |
| - [Toasts](https://primer.style/css/components/toasts) | ||
| - [Flash alerts](https://primer.style/css/components/alerts) | ||
| - [Popovers](https://primer.style/css/components/popover) | ||
| - Toasts |
There was a problem hiding this comment.
I think we should either add a link or remove this until we add toasts to PRC or PVC
Co-authored-by: Mike Perrotti <mperrotti@github.com>
Co-authored-by: Mike Perrotti <mperrotti@github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is for a few small improvements to docs as well as new usage docs for popover and button group
MVP docs (description + usage) added for
Removed outdated button usage UI pattern docs, moved relevant info into the button component docs
Cleaned up some minor things and updated URLS