Conversation
jattasNI
left a comment
jattasNI
left a comment
There was a problem hiding this comment.
Just reviewed the rendered button story and its content, not the new blocks. Might be worthwhile to meet to discuss the vision some more: I like some of the changes but a lot of it feels harder to use to me (possibly due to unfamiliarity or due to me not being the target user).
| submitting a form, opening a dialog, canceling an action, or performing a delete | ||
| operation. _Source: [W3C](https://www.w3.org/WAI/ARIA/apg/patterns/button/)_ | ||
|
|
||
| <Canvas of={buttonStories.outlineButton} /> |
There was a problem hiding this comment.
I don't like that we're getting rid of the canvas. My primary workflow with storybook is putting the control in a certain state and looking at the rendered control or generated code. To be able to do that without crazy scrolling I need the canvas and controls to be near each other. Right now this randomly controls the Outline button canvas below which is too far away to be noticeable.
| operation. _Source: [W3C](https://www.w3.org/WAI/ARIA/apg/patterns/button/)_ | ||
|
|
||
| <Canvas of={buttonStories.outlineButton} /> | ||
| <TableOfContents |
There was a problem hiding this comment.
I was a bit confused by the visual design of the ToC initially. Is it a breadcrumb with arrows instead of slashes between the items? Is it links with arrows next to them? What does the arrow signify again? (My first guess was external link but I guess not). Why isn't there an arrow next to the last link?
I sort of figured out answers to those by playing with it but I'm not sure I see how this is better than the ToC we had before.
| /> | ||
|
|
||
| ## API | ||
| ## Implementation & API |
There was a problem hiding this comment.
Do you know the rationale behind adding the "Implementation" heading? I don't think we generally want to be adding implementation details in storybook.
| - Primary Actions: Use buttons for primary actions that users need to take, such | ||
| as submitting a form, saving changes, or proceeding to the next step. | ||
| - Call to Action (CTA): Buttons are ideal for CTAs that encourage users to take | ||
| a specific action, like “Sign Up,” “Buy Now,” or “Learn More.” |
There was a problem hiding this comment.
This list seems incomplete. You use buttons for secondary and tertiary actions too (by my definition, "OK" is a primary action while "Cancel" would be a secondary action). And in toolbars and many other situations where there is no hierarchy/precedence.
I was also expecting the list to direct me to how to configure the button for each of these use cases but "CTA" isn't used elsewhere in the document and this use of "Primary" doesn't tie back to our appearance-variant='primary'.
After reading a bit more would it make more sense for this to correspond to the numbered sub-headings under Appearance & Styling? Or just remove this list and let users discover the info below?
|
|
||
| ## Alternatives & Related | ||
|
|
||
| ### Alternative to Consider |
There was a problem hiding this comment.
I personally liked having the alternatives near the top because it quickly redirects users to the right component before they spend time reading the whole document.
| Primary outline | ||
| </NimbleButton> | ||
| <NimbleButton appearance="block" appearanceVariant="primary"> | ||
| Primary outline |
There was a problem hiding this comment.
I'm not sure what's going on with the styling here but I don't think black text on a dark grey background is correct.
| @@ -1,45 +1,269 @@ | |||
| import { Canvas, Meta, Controls, Title } from '@storybook/addon-docs/blocks'; | |||
There was a problem hiding this comment.
In my browser (macOS Firefox) the page load time seems not great.
Warm refresh of the production storybook is <1 second: https://nimble.ni.dev/storybook/?path=/docs/components-button--docs
But warm refresh of this page is 6-8 seconds: https://60e89457a987cf003efc0a5b-mgbjyxprll.chromatic.com/?path=/docs/components-button--docs
It's possible this is due to the chromatic URL / pre-deployment build but I'm worried the page might be heavier than before.
| </div> | ||
| </ExampleContainer> | ||
|
|
||
| #### Unmistakably Clear |
There was a problem hiding this comment.
I think I need to hear the pitch for changing the tone to be more "marketing-y". "Unmistakably clear" / "The Stand Out" / "ideal when the Ghost button just isn't enough". I'm not getting a lot of value out of it.
|
|
||
| <DocsGrid columns={2} minColumnWidth={300}> | ||
| <GuidelinesDo title="Create a Clear Button Hierarchy" | ||
| copy="Combine one or more Ghost button(s) with another single button of the available button styles (Outline, Block, Primary or Accent) to establish a clear hierarchy of actions, with the most critical action belonging to the non-ghost button of the set. "> |
There was a problem hiding this comment.
This guidance says to combine ghost with a single button of another style but the code combines it with multiple buttons of different styles.
| href="?path=/docs/components-anchor-button--docs" | ||
| title="Anchor Button" | ||
| > | ||
| Additional context or a short description can go here. |
There was a problem hiding this comment.
We'll want to populate this placeholder content.
2 participants
Pull Request
🤨 Rationale
Initial implementation of an updated visual design for Storybook docs.
👩💻 Implementation
Buttonstory as an example🧪 Testing
✅ Checklist