-
Notifications
You must be signed in to change notification settings - Fork 186
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
Added docs and changelog to main menu #947
Conversation
WalkthroughThe changes in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Sidebar
participant Navigation
User->>Sidebar: Click on "Docs"
Sidebar->>Navigation: Open GitBook documentation page
User->>Sidebar: Click on "Changelog"
Sidebar->>Navigation: Open GitHub releases page
User->>Sidebar: Click on "Support"
Sidebar->>Navigation: Open GitHub issues page
User->>Sidebar: Click on other item
Sidebar->>Navigation: Navigate to specified path
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
Client/src/Components/Sidebar/index.jsx (2)
76-77
: Yo, we're droppin' some knowledge bombs in this menu!These new menu items for Docs and Changelog are comin' in clutch, just like when you're nervous but on the surface you look calm and ready. They're fittin' right in with the rest of the crew.
But yo, I got a little somethin' to add. We might wanna consider sortin' these menu items alphabetically. It ain't a big deal, but it could make it easier for users to find what they're lookin' for, you feel me?
Here's how we could sort it:
const other = [ + { name: "Changelog", path: "changelog", icon: <ChangeLog /> }, + { name: "Docs", path: "docs", icon: <Docs /> }, { name: "Settings", path: "settings", icon: <Settings /> }, { name: "Support", path: "support", icon: <Support /> }, - { name: "Docs", path: "docs", icon: <Docs /> }, - { name: "Changelog", path: "changelog", icon: <ChangeLog /> }, ];
505-531
: Yo, this click handler's got more branches than Eminem's got rhymes!I see what you're doin' here, openin' up them external links for docs and changelog. That's dope, givin' users quick access to the good stuff. But check it, this function's startin' to look like a plate of mom's spaghetti - all tangled up.
How about we clean it up a bit? We could use a switch statement or maybe even a lookup object to make it cleaner than Eminem's closet. Here's a idea to chew on:
const externalLinks = { support: "https://github.com/bluewave-labs/bluewave-uptime/issues", docs: "https://bluewavelabs.gitbook.io/uptime-manager", changelog: "https://github.com/bluewave-labs/bluewave-uptime/releases" }; const handleClick = (path) => { if (externalLinks[path]) { window.open(externalLinks[path], "_blank", "noreferrer"); } else { navigate(`/${path}`); } };Then you can use it like this in your
onClick
:onClick={() => handleClick(item.path)}This way, it's easier to add new external links in the future without makin' your onClick look like a rap battle. What do you think? Ready to drop this mic?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (2)
Client/src/assets/icons/changeLog.svg
is excluded by!**/*.svg
Client/src/assets/icons/docs.svg
is excluded by!**/*.svg
📒 Files selected for processing (1)
- Client/src/Components/Sidebar/index.jsx (3 hunks)
🧰 Additional context used
🔇 Additional comments (1)
Client/src/Components/Sidebar/index.jsx (1)
44-45
: Yo, these new imports are straight fire!Them new icons for ChangeLog and Docs are comin' in hot, just like mom's spaghetti. They're followin' the same flow as the other imports, so we're keepin' it real consistent. That's what I'm talkin' about!
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.
Nice work 👍 I think we can do some minor refactoring to make the code a bit more maintainable and readable, and it would be nice to have the Settings link be the first in the list. Please see comments for details.
Thanks for your hard work and contribution, much appreciated!
Thanks for the feedback. That looks cleaner. |
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.
Awesome, looks good now 👍 Thanks for making the changes!
My pleasure, looks good to me now 🚀 |
output.compress-video-online.com.mp4
#942