-
Notifications
You must be signed in to change notification settings - Fork 1.4k
docs: update side nav #9178
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
docs: update side nav #9178
Conversation
|
Build successful! 🎉 |
snowystinger
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 don't think I'm a fan of the Search opening directly to the getting started, I think it'd be more useful to open to the components or just All
Where are you seeing that? Is it when you're switching libraries? The search should only open to "overview" if you're in that section. Otherwise, if you're on a component page and you open the search, it should open to "components". I've updated the code a bit so if you're on the react aria homepage and click on the "Explore components" button, it'll open to the components page |
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.
a more general question for the team I guess, but with regards to the internationalized pages, should they still redirect to /internationalized or say under react-aria ? I wonder if it is jarring to have the side nav change out from under you
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 i wasn't sure what the url is going to look like. i assumed it would be something like 'react-aria.adobe.com/internationalized/...".
it might be weird with the side nav changing but at the same time, it is a separate library so i feel like it shouldn't include the react-aria stuff
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 can debate whether it should be in the React Aria sidenav I guess. I think we wanted to make it a little more visible. Right now it's a little strange that it is there but then to get back to react aria you need to use the search menu. Ideally whatever we decide would be symmetrical I think.
packages/dev/s2-docs/src/Header.tsx
Outdated
| <BetaApp /> | ||
| <Text>Beta Preview</Text> | ||
| </Badge> | ||
| <ActionButton |
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.
Are these buttons supposed to work on the build? I noticed they strip out the commit hash when clicking them so the page doesn't actually show up
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 should these buttons appear somewhere in the mobile experience? Or should the header logo button bring you back? I guess maybe the search menu is good enough on its own
|
Build successful! 🎉 |
| })}> | ||
| <div className={style({justifySelf: 'start'})}> | ||
| <ActionButton | ||
| <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.
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.
hah time to add LinkActionButton? 😂
| </Badge> | ||
| <Link className={renderProps => linkStyles({...renderProps})} href={docs} ref={docsRef} style={pressScale(docsRef)} >Docs</Link> | ||
| <Link className={renderProps => linkStyles({...renderProps})} href={release} ref={releasesRef} style={pressScale(releasesRef)} >Releases</Link> | ||
| <Link className={renderProps => linkStyles({...renderProps})} href={blog} target={subdirectory === 's2' ? '_blank' : ''} rel="noopener noreferrer" ref={blogRef} style={pressScale(blogRef)} >Blog</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.
We might want to use the same Link + styles for the Github link
|
Build successful! 🎉 |
|
Build successful! 🎉 |
|
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.
Looks great!
One nice thing to add would be for the current category in the sidenav to automatically expand. Right now if you navigate to a page in Utilities for example and then reload the page, Components is expanded but not Utilities. We could probably default the state to the current page's category.
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 can debate whether it should be in the React Aria sidenav I guess. I think we wanted to make it a little more visible. Right now it's a little strange that it is there but then to get back to react aria you need to use the search menu. Ideally whatever we decide would be symmetrical I think.
| let subdirectory = 's2'; | ||
| if (library === 'internationalized' || library === 'react-aria') { | ||
| // the internationalized library has no homepage so i've chosen to route it to the react aria homepage | ||
| subdirectory = 'react-aria'; |
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 is another oddity about having it under react-aria 🤷
| })}> | ||
| <div className={style({justifySelf: 'start'})}> | ||
| <ActionButton | ||
| <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.
hah time to add LinkActionButton? 😂
|
|
||
| return ( | ||
| <DialogTrigger isOpen={searchOpen} onOpenChange={() => {openSearchMenu();}} > | ||
| <Pressable><span role="button" className="no-underline bg-white/60 border border-black/10 bg-clip-padding text-base md:text-lg font-bold text-slate-800 px-8 py-3 rounded-full backdrop-saturate-150 backdrop-brightness-125 transition hover:bg-white/60 focus-ring dark:outline-white outline-offset-2 active:scale-95">Explore Components</span></Pressable> |
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.
Any reason it can't be a RAC Button?
also tiny nit: add cursor-default so the text selection cursor doesn't come up
| </ModalOverlay> | ||
| </DialogTrigger> | ||
| ); | ||
| } |
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 might need to adjust the view transition a little. Noticed that the search field doesn't fade out with the rest of the modal - I guess it's assuming there is a search on the page to transition to but there isn't in this case.
* update getting started to overview * hide side nav for release notes * hide blog from sidenav * add export group to add internationalize to react aria side nav * remove releases from react spectrum side nav * update side nav to include internationalized in react aria * explore component opens search menu * fix routing? * fix some stuff from merge conflict * pls fix lint * update github logo to link * fix lint * add back beta badge * fix lint

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