-
Notifications
You must be signed in to change notification settings - Fork 1.4k
chore: add links to old docs for hooks we haven't migrated yet #9203
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
Conversation
| import {InlineAlert, Heading, Content} from '@react-spectrum/s2' | ||
|
|
||
| export const tags = ['hint', 'popup', 'info']; | ||
| export const relatedPages = [{'title': 'useTooltipTrigger', 'url': 'https://react-spectrum.adobe.com/react-aria/useTooltipTrigger.html'}, {'title': 'useTooltip', 'url': 'https://react-spectrum.adobe.com/react-aria/useTooltip.html'}]; |
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.
useTooltip doesn't exist, it is part of the useTooltipTrigger page, I'll go and update this
LFDanLu
left a comment
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.
LGTM, IMO the mobile experience is fine, no need to add more content since related pages is just a collection of links, hopefully the link titles are explanatory enough
|
Build successful! 🎉 |
|
Build successful! 🎉 |
## API Changes
@react-spectrum/s2/@react-spectrum/s2:EditableCell+EditableCell {
+ action?: string | FormHTMLAttributes<HTMLFormElement>['action']
+ align?: 'start' | 'center' | 'end' = 'start'
+ children: ReactNode
+ colSpan?: number
+ id?: Key
+ isSaving?: boolean
+ onCancel?: () => void
+ onSubmit?: (FormEvent<HTMLFormElement>) => void
+ renderEditing: () => ReactNode
+ showDivider?: boolean
+ textValue?: string
+} |
reidbarber
left a comment
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 good with it in its current form, but I have two design opinions:
- Should these be external links? (i.e. have the arrow icon and open in a new tab?)
- For mobile, should it be a bulleted list?
devongovett
left a comment
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.
Ok for testing
| <H2>Related pages</H2> | ||
| <Ul> | ||
| <H2 id="related-pages">Related pages</H2> | ||
| <Ul className={style({listStyleType: 'none'})}> |
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.
why no bullets?
| })}> | ||
| <H2>Related pages</H2> | ||
| <Ul> | ||
| <H2 id="related-pages">Related pages</H2> |
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.
why id? I think this should be auto-generated?
Closes
Adds links to our RAC pages for the underlying hooks used to build those components.
Is the mobile experience ok? it's a bit bare.
✅ Pull Request Checklist:
📝 Test Instructions:
🧢 Your Project: