-
Notifications
You must be signed in to change notification settings - Fork 1.4k
docs: S2 docs newest testing fixes #9241
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
|
Build successful! 🎉 |
|
Build successful! 🎉 |
|
Build successful! 🎉 |
|
Build successful! 🎉 |
|
|
||
| export const section = 'Date and Time'; | ||
| export const group = 'Internationalized'; | ||
| export const description = 'Calendar systems for international date calculations.'; |
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.
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.
Also putting here since there isn't a good place to comment this but I noticed that some of the descriptions of the aria utilities just say "Implementing collections in React Aria" which doesn't seem right but is probably existing:
The rest of the descriptions seem ok to me, but just wanted to note that some are different from what we had in https://react-spectrum.adobe.com/react-spectrum/index.html (e.g. TagGroup, Tabs, ActionButton, etc) and are a bit long compared to those (and thus end up being cut off). Happy to keep them as is, but just wanted to note the differences
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.
Now that we have descriptions for each component, could we have the descriptions start from right beneath the component name?
I think that makes sense, we'd need to fix this in S2 so we can handle that separately.
Also putting here since there isn't a good place to comment this but I noticed that some of the descriptions of the aria utilities just say "Implementing collections in React Aria" which doesn't seem right but is probably existing:
Will push a fix.
The rest of the descriptions seem ok to me, but just wanted to note that some are different from what we had in https://react-spectrum.adobe.com/react-spectrum/index.html (e.g. TagGroup, Tabs, ActionButton, etc) and are a bit long compared to those (and thus end up being cut off). Happy to keep them as is, but just wanted to note the differences
I was looking at the JSDocs and forgot about those. I'll update some of these to be shorter!
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.
do you have any examples where I could test this one?
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.
In
the [selection guide](selection.md?component=GridList#selection-behavior) link.
| function isExternalUrl(url: string): boolean { | ||
| return url.startsWith('http://') || url.startsWith('https://'); | ||
| } |
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 wonder if we'll need to change this when we have the 2 separate domains with links between each. I guess I'd be fine if it would open a new tab but happy for the team to weigh in. I could see an argument that links pointing to either domain should keep you in the same tab
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 we'll need to update this. All links will be absolute after my PR is merged. There is also a similar utility in Link.tsx already (which I updated in my PR).
|
Build successful! 🎉 |
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.
We should standardize on whether the descriptions start with "A component name does X" or just "Does X" (I would vote for the latter). Right now it's a mix.
Also would be nice to make the description lengths more uniform. Right now some are longer than others which looks a little strange.
|
Build successful! 🎉 |
|
Build successful! 🎉 |

✅ Pull Request Checklist:
📝 Test Instructions:
🧢 Your Project: