-
Notifications
You must be signed in to change notification settings - Fork 534
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
feat: Added support to sidebar #2119
Conversation
|
@harshsbhat is attempting to deploy a commit to the Unkey Team on Vercel. A member of the Team first needs to authorize it. |
📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThe changes introduce a new Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
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
CodeRabbit Configuration File (
|
Thank you for following the naming conventions for pull request titles! 🙏 |
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 (3)
apps/dashboard/components/dashboard/feedback-component.tsx (3)
1-22
: Looks good! Consider grouping the imports for better readability.The imports are correctly used and cover all the necessary dependencies. However, consider grouping them based on their origin (e.g., separate groups for
@/components
,@/lib
, external libraries) to improve readability.
23-53
: The component logic is well-structured. Consider adding error handling for thetrpc
mutation.The component manages the dialog state and form handling correctly. The usage of
react-hook-form
andzod
for form validation is a good practice.However, consider adding error handling for the
trpc
mutation to handle potential network or server errors gracefully. You can use theonError
callback of theuseMutation
hook to display an error message to the user.
55-157
: The JSX is properly structured. Consider extracting the form fields into separate components.The JSX correctly renders the feedback dialog and form. The usage of custom UI components enhances the consistency and reusability of the code.
However, the form fields (
issueType
,severity
,message
) are quite verbose and contain similar logic. Consider extracting them into separate components (e.g.,IssueTypeField
,SeverityField
,MessageField
) to improve maintainability and reduce duplication.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- apps/dashboard/app/(app)/desktop-sidebar.tsx (5 hunks)
- apps/dashboard/components/dashboard/feedback-component.tsx (1 hunks)
Additional comments not posted (5)
apps/dashboard/app/(app)/desktop-sidebar.tsx (5)
2-2
: LGTM!The import statement for the
Feedback
component is correctly added.
41-41
: LGTM!The modification to the
icon
property in theNavItem
type is a good change. It allows using custom React components as icons, providing more flexibility in the sidebar navigation.
50-65
: LGTM!The
DiscordIcon
component is correctly implemented as a functional component. The SVG code for the Discord logo is properly included, and the component can be used in other parts of the codebase.
148-153
: LGTM!The "Discord" navigation item is correctly added to the
resourcesNavigation
array. It uses theDiscordIcon
component as the icon and links to the external Discord URL. Theexternal
property is properly set totrue
to open the link in a new tab.
202-204
: LGTM!The
Feedback
component is correctly included as a list item within the "Resources" section of the sidebar. This provides users with convenient access to the feedback functionality.
I have created a new component for the sidebar because it was not allowing me to use |
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.
@chronark I have pushed the requested changes for this one too |
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)
apps/dashboard/components/dashboard/command-menu.tsx (2)
10-10
: Consider removing unnecessary React importThe explicit import of React on line 12 may not be necessary if you're using a modern React setup with the new JSX transform. This transform allows you to use JSX without importing React in scope.
If you're using React 17 or later with a compatible build setup, you can safely remove this line:
-import React from "react";
However, ensure that your project is configured to use the new JSX transform before making this change.
Also applies to: 12-13
90-96
: LGTM: FeedbackCommand implementationThe FeedbackCommand component is implemented correctly, integrating the FeedbackComponent within a CommandItem. This aligns with the PR objectives of improving support accessibility.
Consider making the component more flexible by accepting props:
-const FeedbackCommand: React.FC = () => { +const FeedbackCommand: React.FC<{ variant?: string }> = ({ variant = 'command' }) => { return ( <CommandItem> - <FeedbackComponent variant="command" /> + <FeedbackComponent variant={variant} /> </CommandItem> ); };This change would allow for easier customization if needed in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- apps/dashboard/app/(app)/desktop-sidebar.tsx (5 hunks)
- apps/dashboard/components/dashboard/command-menu.tsx (3 hunks)
- apps/dashboard/components/dashboard/feedback-component.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/dashboard/app/(app)/desktop-sidebar.tsx
- apps/dashboard/components/dashboard/feedback-component.tsx
🔇 Additional comments not posted (2)
apps/dashboard/components/dashboard/command-menu.tsx (2)
43-43
: LGTM: FeedbackCommand added to Help groupThe addition of the FeedbackCommand to the Help group in the CommandMenu aligns well with the PR objectives. This change improves user accessibility to support features, making them more prominent and easier to find.
Line range hint
1-96
: Verify impact of simplified feedback mechanismThe changes have significantly simplified the feedback collection process by removing the complex Feedback component (with dialog, form validation, and submission handling) in favor of the simpler FeedbackCommand. While this aligns with the PR objectives of improving support accessibility, it's important to ensure that this simplification doesn't negatively impact the quality of feedback collected or the user experience.
Could you provide more information on how the new FeedbackComponent handles the feedback collection process? Specifically:
- How is the feedback submitted now?
- Is there still form validation in place?
- How does this change impact the user experience when providing feedback?
To help verify the changes, please run the following script:
This will help ensure that all references to the old Feedback component have been updated and provide insight into the implementation of the new FeedbackComponent.
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
why not? |
Yeah I know it's working in prod. I just wanted to update you that I am still working on it. So you don't accidentally keep cloning my branch it just to realise this error. |
…ing the arrows and enter key on command
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: 1
🧹 Outside diff range and nitpick comments (2)
apps/dashboard/components/dashboard/command-menu.tsx (2)
43-43
: LGTM: FeedbackCommand added to Help groupThe addition of
<FeedbackCommand />
to the Help group in the CommandMenu is a good implementation of the PR objectives. It improves the accessibility of the support features as intended.Consider adding a comment above the FeedbackCommand to briefly explain its purpose, which could be helpful for future maintenance. For example:
+ {/* Feedback option for users to report issues or provide suggestions */} <FeedbackCommand />
Line range hint
1-96
: Overall implementation aligns well with PR objectivesThe changes in this file successfully address the PR objectives of improving support accessibility in the sidebar. Key points:
- The new FeedbackCommand component simplifies the feedback mechanism as intended.
- The addition of the feedback option to the Help group in the CommandMenu improves discoverability.
- The overall structure of the CommandMenu remains clean and maintainable.
These changes should effectively enhance user accessibility to support features, addressing the concerns raised in issue #2095.
To further improve the implementation, consider:
- Ensuring consistent prop naming conventions across components.
- Adding brief comments to explain the purpose of new components or significant changes.
- If not already done, update relevant documentation to reflect these new support options in the sidebar.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
- apps/dashboard/app/(app)/desktop-sidebar.tsx (5 hunks)
- apps/dashboard/components/dashboard/command-menu.tsx (3 hunks)
- apps/dashboard/components/dashboard/feedback-component.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/dashboard/app/(app)/desktop-sidebar.tsx
- apps/dashboard/components/dashboard/feedback-component.tsx
🔇 Additional comments (1)
apps/dashboard/components/dashboard/command-menu.tsx (1)
10-10
: LGTM: Import statements updated correctlyThe changes to the import statements are consistent with the new implementation of the feedback feature. The addition of
BookOpen
andLucideIcon
imports, along with theFeedbackComponent
, aligns well with the simplified feedback mechanism mentioned in the PR summary.Also applies to: 12-13
…tered using the arrows and enter key on command" This reverts commit dc6183f.
Done. I think this is ready to review now. I have made all heights and widths consistent. |
You will probably need to submit the form to check if it is working I was not able to do so. As I don't have a Plain API key. It should work tbh. As I haven't made any other changes in the code other than styling |
* Discord dark mode and fixing the link * Requested changes * Requested changes * Lint * Adding support to sidebar * remove unused imports * change the docs icon to book and remove repeating code * Consistent size * Making sure the Feedback is clickable, the feedback can be entered using the arrows and enter key on command * Revert "Making sure the Feedback is clickable, the feedback can be entered using the arrows and enter key on command" This reverts commit dc6183f. * pnpm-lock file * Fix --------- Co-authored-by: Andreas Thomas <dev@chronark.com>
* Discord dark mode and fixing the link * Requested changes * Requested changes * Lint * Adding support to sidebar * remove unused imports * change the docs icon to book and remove repeating code * Consistent size * Making sure the Feedback is clickable, the feedback can be entered using the arrows and enter key on command * Revert "Making sure the Feedback is clickable, the feedback can be entered using the arrows and enter key on command" This reverts commit dc6183f. * pnpm-lock file * Fix --------- Co-authored-by: Andreas Thomas <dev@chronark.com>
What does this PR do?
Fixes #2095
Adds the support options to side bar
Type of change
Checklist
Required
pnpm build
pnpm fmt
console.logs
git pull origin main
Appreciated
Summary by CodeRabbit