Skip to content

Conversation

@Lymah123
Copy link
Contributor

@Lymah123 Lymah123 commented Nov 8, 2025

Summary

Add fileDialogOpen guard flag to select-file-or-directory IPC handler to prevent
race condition where rapid double-clicks would spawn multiple overlapping
file picker dialogs. Uses try-finally to ensure flag is always reset.

Also add .cargo/ to .gitignore to exclude local cargo config.

Type of Change

  • Feature
  • Bug fix
  • Refactor / Code quality
  • Performance improvement
  • Documentation
  • Tests
  • Security fix
  • Build / Release
  • Other (specify below)

AI Assistance

  • This PR was created or reviewed with AI assistance

Testing

Related Issues

Relates to #5527
Discussion: LINK (if any)

Screenshots/Demos (for UX changes)

Before:
image

After:
Screenshot (30)
Screenshot (31)

Submitting a Recipe?

Email:

Copilot AI review requested due to automatic review settings November 8, 2025 12:22
@Lymah123 Lymah123 requested a review from a team as a code owner November 8, 2025 12:22
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request includes improvements to file dialog handling in the desktop application, adds a new community content submission, and updates the .gitignore file.

  • Prevents multiple simultaneous file dialogs using a flag with try-finally cleanup
  • Adds a community blog post submission about managing goose configurations
  • Ignores the .cargo/ directory in version control

Reviewed Changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.

File Description
ui/desktop/src/main.ts Adds protection against multiple simultaneous file dialogs with proper cleanup using try-finally pattern
documentation/src/pages/community/data/community-content.json Adds a community blog post entry about managing goose configurations
.gitignore Adds .cargo/ directory to ignored paths

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 303 to 312
"title": "Managing goose Configurations Across Multiple Projects",
"author": {
"name": "Halimah",
"handle": "lymah123"
},
"type": "blog",
"url": "https://dev.to/lymah/managing-goose-configurations-across-multiple-projects-58i3",
"thumbnail": "https://i.imgur.com/VvM8sMR.png",
"submittedDate": "2025-10-30",
"tags": ["hacktoberfest"],
Copy link

Copilot AI Nov 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This entry appears to be a duplicate of the existing entry at lines 273-286. Both entries have the same title, author, URL, thumbnail, and submitted date. This duplicate should be removed to avoid redundant content in the community submissions list.

Suggested change
"title": "Managing goose Configurations Across Multiple Projects",
"author": {
"name": "Halimah",
"handle": "lymah123"
},
"type": "blog",
"url": "https://dev.to/lymah/managing-goose-configurations-across-multiple-projects-58i3",
"thumbnail": "https://i.imgur.com/VvM8sMR.png",
"submittedDate": "2025-10-30",
"tags": ["hacktoberfest"],
"title": "Managing Goose Configurations Across Multiple Projects: A Practical Guide",
"author": {
"name": "Viet Bui",
"handle": "vietbui1999ru"
},
"type": "blog",
"url": "https://dev.to/vietbui1999ru/managing-goose-configurations-across-multiple-projects-a-practical-guide-48ei",
"thumbnail": "https://imgur.com/a/ubyYbVJ",
"submittedDate": "2025-10-31",
"tags": [
"hacktoberfest"
],

Copilot uses AI. Check for mistakes.
@Lymah123
Copy link
Contributor Author

Lymah123 commented Nov 8, 2025

Hi @DOsinga! I will be waiting for your review.

Copy link
Collaborator

@DOsinga DOsinga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for jumping on this. I think we need to solve this at the caller level though

if we add functonality to the selectFileOrDirectory calls (there's two, but we only really care about the one in ChatInput.tsx) we can just dissable the button while the modal is open.

that would be a better experience since with the current proposed fix, if we have two goose windows and you select files from both, as soon as the first one closes, the second one would allow you to open an extra window, but also, disabling a button is better than just doing nothing because that other window is already open

@Lymah123
Copy link
Contributor Author

Lymah123 commented Nov 11, 2025

Hi!
Just to confirm: would you prefer we handle this by disabling the select button in ChatInput.tsx while the dialog is open, instead of managing it in the Electron main process? Want to make sure I'm taking the right approach, thanks! @DOsinga

@DOsinga
Copy link
Collaborator

DOsinga commented Nov 11, 2025

Hi! Just to confirm: would you prefer we handle this by disabling the select button in ChatInput.tsx while the dialog is open, instead of managing it in the Electron main process? Want to make sure I'm taking the right approach, thanks! @DOsinga

yes, I think that would work better - thanks

@Lymah123
Copy link
Contributor Author

Hi! Just to confirm: would you prefer we handle this by disabling the select button in ChatInput.tsx while the dialog is open, instead of managing it in the Electron main process? Want to make sure I'm taking the right approach, thanks! @DOsinga

yes, I think that would work better - thanks

Alright. I will work on the requested changes. Thanks

"hacktoberfest"
],
"hacktoberfest": true
},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes don't seem relevant to the intended scope of the PR. Are they left in unintentionally?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will remove those files once I am done making the requested changes. They were from my past PRs.

@Lymah123 Lymah123 force-pushed the bug branch 2 times, most recently from f8fed0c to a5434b9 Compare November 13, 2025 02:55
@Lymah123 Lymah123 requested a review from DOsinga November 13, 2025 02:56
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file shouldn't be deleted; just your changes need to be reverted

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Thanks

Copy link
Collaborator

@Abhijay007 Abhijay007 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

commented above

@Lymah123 Lymah123 force-pushed the bug branch 2 times, most recently from 419d128 to 3691122 Compare November 14, 2025 16:22
const [displayValue, setDisplayValue] = useState(initialValue); // For immediate visual feedback
const [isFocused, setIsFocused] = useState(false);
const [pastedImages, setPastedImages] = useState<PastedImage[]>([]);
const [isFileDialogOpen, setIsFileDialogOpen] = useState(false); // NEW: Track file dialog state
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just removing extra comments that we may not need

Suggested change
const [isFileDialogOpen, setIsFileDialogOpen] = useState(false); // NEW: Track file dialog state
const [isFileDialogOpen, setIsFileDialogOpen] = useState(false);

<DirSwitcher className="mr-0" />
<div className="w-px h-4 bg-border-default mx-2" />

{/* Attach button now disables when dialog is open */}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think we may not need this comment either

Suggested change
{/* Attach button now disables when dialog is open */}

.gitignore Outdated

# Goose self-test artifacts
gooseselftest/
.cargo/
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can remove this change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants