Skip to content

Conversation

@benjamindrussell
Copy link

Summary

Added state to track file picker status, disabled the button when file picker open.
Also, not mentioned in the original issue but the same behavior occurs for the directory chooser as well so I went ahead and fixed it.
Added state to track directory chooser status, disabled the button when directory chooser open.

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

Manual Testing

Related Issues

Relates to #5527
Discussion: LINK (if any)

Screenshots/Demos (for UX changes)

Before:
image

After:
image

image

Submitting a Recipe?

Email:

Copilot AI review requested due to automatic review settings November 27, 2025 18:47
Copilot finished reviewing on behalf of benjamindrussell November 27, 2025 18:48
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 PR fixes a bug where users could open multiple file picker or directory chooser dialogs by rapidly clicking the corresponding buttons. The fix adds state tracking to prevent re-opening dialogs while one is already active.

Key changes:

  • Added state management to track when pickers/choosers are open
  • Disabled buttons and added visual feedback (opacity) during picker/chooser operations
  • Wrapped async operations in try-finally blocks to ensure cleanup

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
ui/desktop/src/components/bottom_menu/DirSwitcher.tsx Adds isDirectoryChooserOpen state, disables button during directory selection, and prevents tooltip from showing when chooser is active
ui/desktop/src/components/ChatInput.tsx Adds isFilePickerOpen state and disables file attachment button during file selection

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

@benjamindrussell benjamindrussell force-pushed the 5527-multiple-file-popups branch from 3bcc306 to 70a4c2e Compare November 27, 2025 18:52
@benjamindrussell benjamindrussell requested a review from a team as a code owner November 27, 2025 18:52
@angiejones angiejones force-pushed the 5527-multiple-file-popups branch from 70a4c2e to f30a0f6 Compare November 28, 2025 00:41
@angiejones angiejones requested review from Copilot and removed request for a team November 28, 2025 00:41
Copilot finished reviewing on behalf of angiejones November 28, 2025 00:43
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

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@The-Best-Codes
Copy link
Collaborator

The-Best-Codes commented Nov 28, 2025

FYI, there is a PR (#5640) for the issue already. The author of that PR was the first to begin work on issue #5527, and they are assigned to it.

Comment on lines +24 to +28
if (isDirectoryChooserOpen) {
event.preventDefault();
event.stopPropagation();
return;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just out of curiosity, can you explain why just using return; won't work here?

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, returning only exits the handler, it doesn’t cancel the browser’s default behavior or stop the event from bubbling. We could remove preventDefault and add a type="button" to the button, since the default type is submit. TBH it would probably still work fine with just return.

@benjamindrussell
Copy link
Author

FYI, there is a PR (#5640) for the issue already. The author of that PR was the first to begin work on issue #5527, and they are assigned to it.

Ah I see, I'm happy to change the scope of this PR to just fix the behavior of the directory chooser if necessary.

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.

2 participants