-
-
Notifications
You must be signed in to change notification settings - Fork 6
feat(mobile): Enable attachment button functionality #309
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,7 +18,11 @@ import { History } from '@/components/history' | |
| import { MapToggle } from './map-toggle' | ||
| import { ModeToggle } from './mode-toggle' | ||
|
|
||
| export const MobileIconsBar: React.FC = () => { | ||
| interface MobileIconsBarProps { | ||
| onAttachmentClick: () => void; | ||
| } | ||
|
|
||
| export const MobileIconsBar: React.FC<MobileIconsBarProps> = ({ onAttachmentClick }) => { | ||
| const [, setMessages] = useUIState<typeof AI>() | ||
| const { clearChat } = useActions() | ||
|
|
||
|
|
@@ -45,7 +49,7 @@ export const MobileIconsBar: React.FC = () => { | |
| <Button variant="ghost" size="icon"> | ||
| <TentTree className="h-[1.2rem] w-[1.2rem] transition-all rotate-0 scale-100" /> | ||
| </Button> | ||
| <Button variant="ghost" size="icon"> | ||
| <Button variant="ghost" size="icon" onClick={onAttachmentClick}> | ||
| <Paperclip className="h-[1.2rem] w-[1.2rem] transition-all rotate-0 scale-100" /> | ||
| </Button> | ||
|
Comment on lines
+52
to
54
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Icon-only buttons should provide an accessible name. The paperclip button currently lacks an SuggestionAdd an accessible label to the button: <Button variant="ghost" size="icon" onClick={onAttachmentClick} aria-label="Attach file">
<Paperclip className="h-[1.2rem] w-[1.2rem] transition-all rotate-0 scale-100" />
</Button>Alternatively, add visually hidden text via an Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this change. |
||
| <Button variant="ghost" size="icon"> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| ⚠ Port 3000 is in use, using available port 3001 instead. | ||
| ▲ Next.js 15.3.3 (Turbopack) | ||
| - Local: http://localhost:3001 | ||
| - Network: http://192.168.0.2:3001 | ||
| - Environments: .env.local, .env | ||
|
|
||
| ✓ Starting... | ||
|
Comment on lines
+1
to
+7
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Committing dev logs is a maintainability and hygiene issue. This file appears to contain local development diagnostics and should not be versioned. It risks repository noise and potential leakage of environment info. Please remove it and add it to SuggestionRemove Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion |
||
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.
🧹 Nitpick | 🔵 Trivial
Consider refactoring to eliminate code duplication.
The imperative handle directly calls
fileInputRef.current?.click(), duplicating the logic in the internalhandleAttachmentClickfunction (lines 61-63). Consider having the imperative handle call the internal function instead for better maintainability.Apply this diff:
useImperativeHandle(ref, () => ({ handleAttachmentClick() { - fileInputRef.current?.click() + handleAttachmentClick() } }));Note: This change requires moving the internal
handleAttachmentClickfunction definition before theuseImperativeHandlecall to avoid referencing it before declaration.🤖 Prompt for AI Agents