-
Notifications
You must be signed in to change notification settings - Fork 535
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
Update component docs (AnchoredOverlay, Avatar, AvatarStack, Box, BranchName, Breadcrumbs) #1702
Conversation
🦋 Changeset detectedLatest commit: 94b8ffe The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
size-limit report 📦
|
…s into update-component-docs-2
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.
So much better! 🎨 🥇
side note: It would be awesome if we could get syntax highlighting for our Type column in these tables.
name="overlayProps" | ||
type={ | ||
<> | ||
Partial<<Link href="/Overlay#props">OverlayProps</Link>> |
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.
✨
@@ -6,9 +6,11 @@ exports[`BranchName renders consistently 1`] = ` | |||
padding: 2px 6px; | |||
font-size: 12px; | |||
font-family: SFMono-Regular,Consolas,"Liberation Mono",Menlo,Courier,monospace; | |||
color: #57606a; | |||
color: #0969da; |
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.
That seems like a pretty drastic change — is that expected?
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.
<> | ||
An override to the internal <InlineCode>renderAnchor</InlineCode> ref that will be used to position the overlay. | ||
When <InlineCode>renderAnchor</InlineCode> is | ||
<InlineCode>null</InlineCode>, this can be used to make an anchor that is detached from <InlineCode> |
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 wording is tricky — unlike most ref props, there's an expectation that the AnchoredOverlay
isn't responsible for setting the value of the ref, right? only that it will use the ref's current value for positioning logic?
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.
Yeah, this one is tricky. I mostly copied this description from the code comments but I agree that it's a bit confusing.
<PropsTableRow | ||
name="alt" | ||
type="string" | ||
defaultValue="''" |
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'm surprised this is ''
and not undefined.
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 this is intentional. alt=''
is treated differently than an undefined alt
. From https://davidwalsh.name/accessibility-tip-empty-alt-attributes:
Images with only visual value should have an empty alt attribute set on them. Omitting the alt attribute makes most screen readers read out the entire image URL and providing an alt attribute when the image is for visual purposes only is just as useless.
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.
TIL, thanks!
…nchName, Breadcrumbs) (#1702) * Update AnchoredOverlay docs * Allow preformatted types in prop table * Add Link and InlineCode to global mdx scope * Update Avatar docs * Update AvatarStack docs * Update Box props * Update branchname docs * Update breadcrumbs docs * Update branchname snapshot * Create polite-trees-wink.md * Add AvatarPair docs
Part of #1701
Previews