-
Notifications
You must be signed in to change notification settings - Fork 216
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: new chat avatar style #308
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Hey @okisdev - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Docstrings: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
@@ -15,6 +18,8 @@ export const ConversationWindow = ({ | |||
}>) => { | |||
const t = useTranslations(); | |||
|
|||
const [currentUseModel] = useAtom(store.currentUseModelAtom); |
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.
suggestion (code_clarification): Consider renaming currentUseModel
to currentUserModel
for clarity.
The name currentUseModel
might be a typo or could be misinterpreted. Renaming it to currentUserModel
would align better with typical naming conventions and improve code readability.
const [currentUseModel] = useAtom(store.currentUseModelAtom); | |
const [currentUserModel] = useAtom(store.currentUseModelAtom); |
onClick={() => onCopy(m.content)} | ||
className='inline-flex items-center space-x-1 rounded-md bg-neutral-200 px-2 py-1 transition duration-300 ease-in-out hover:bg-neutral-400/30 dark:bg-neutral-600 dark:text-neutral-200/90 dark:shadow-lg dark:hover:bg-neutral-700 dark:hover:shadow-xl' | ||
> | ||
<LuClipboardCopy size={10} /> |
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.
suggestion (code_refinement): Consider using a larger icon size for better visibility.
The current size of 10 for the clipboard icon might be too small for easy interaction, especially on mobile devices. Increasing the size could enhance usability.
<LuClipboardCopy size={10} /> | |
<LuClipboardCopy size={16} /> |
No description provided.