Skip to content

Conversation

farheen-shaikh530
Copy link

Summary of the Pull Request

This PR introduces a confirmation dialog that allows users to choose between opening the settings JSON file or the containing folder. This enhances user flexibility when accessing configuration files from the terminal.

Motivation

Previously, clicking "Open JSON" would directly open the file. This change adds user choice, improving usability and matching expectations for broader editing or navigation workflows.

References and Relevant Issues

Fixes #14410
Marked as [Good First Issue] in the Microsoft Terminal GitHub repository.

Detailed Description of the Pull Request / Additional comments

•	Modified the sui.OpenJson event handler inside TerminalPage.cpp.
•	Introduced Windows::UI::Popups::MessageDialog to show two options:
•	Open File
•	Open Folder
•	Based on user selection, the appropriate _LaunchSettings() method is called.
•	No breaking changes were introduced.

Validation Steps Performed

•	Verified dialog appears when clicking “Open JSON”
•	Confirmed “Open File” launches the correct settings JSON
•	Confirmed “Open Folder” opens the parent folder using DefaultsFile
•	Ensured no UI crashes or regressions
•	Code compiles and builds cleanly

Contributor License Agreement

@microsoft-github-policy-service agree

I confirm that I am submitting this contribution as an individual, and I own the rights to this code. I am not submitting on behalf of an employer or organization.

PR Checklist

  • Closes #xxx
  • Tests added/passed
  • Documentation updated
    • If checked, please file a pull request on our docs repo and link it here: #xxx
  • Schema updated (if necessary)

@microsoft-github-policy-service microsoft-github-policy-service bot added Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Area-Settings Issues related to settings and customizability, for console or terminal Product-Terminal The new Windows Terminal. labels Jun 21, 2025
@htcfreek
Copy link
Contributor

My thoughts: Why a content dialog and not a button with secondary conmand? A button with secondary command to open the folder feels much user friendlier.

@farheen-shaikh530 farheen-shaikh530 force-pushed the feature/open-json-folder-option branch from af2ff44 to d35ec59 Compare June 27, 2025 06:44
@farheen-shaikh530
Copy link
Author

Hi team, I've cleaned up the PR and removed unrelated .vcxproj changes. Now, it only includes the intended logic for adding the confirmation dialog to choose between opening the settings file or its folder. Ready for your review. Thanks again for the guidance!

@farheen-shaikh530
Copy link
Author

My thoughts: Why a content dialog and not a button with secondary conmand? A button with secondary command to open the folder feels much user friendlier.

Thank you for the feedback!
I agree that using a button with a secondary command could feel more seamless for users.
I initially went with a ContentDialog to align with how other prompts are handled in the Terminal UI and to make the choice explicit when clicking “Open JSON.” That said, I’m open to switching to a secondary command on the existing button if that better matches the UX expectations or existing UI patterns.
Let me know your thoughts — happy to iterate based on the team’s preference!

@htcfreek
Copy link
Contributor

My thoughts: Why a content dialog and not a button with secondary conmand? A button with secondary command to open the folder feels much user friendlier.

Thank you for the feedback!
I agree that using a button with a secondary command could feel more seamless for users.
I initially went with a ContentDialog to align with how other prompts are handled in the Terminal UI and to make the choice explicit when clicking “Open JSON.” That said, I’m open to switching to a secondary command on the existing button if that better matches the UX expectations or existing UI patterns.
Let me know your thoughts — happy to iterate based on the team’s preference!

@lhecker
I can't decide this. Thoughts?

@lhecker lhecker added the Needs-Discussion Something that requires a team discussion before we can proceed label Jun 30, 2025
@carlos-zamora
Copy link
Member

carlos-zamora commented Jul 14, 2025

Hi @farheen-shaikh530. A few comments/questions:

Thanks!

@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Jul 14, 2025
@microsoft-github-policy-service microsoft-github-policy-service bot added the No-Recent-Activity This issue/PR is going stale and may be auto-closed without further activity. label Jul 21, 2025
Copy link
Contributor

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within 7 days of this comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Settings Issues related to settings and customizability, for console or terminal Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something Needs-Discussion Something that requires a team discussion before we can proceed No-Recent-Activity This issue/PR is going stale and may be auto-closed without further activity. Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] Option to auto trim leading/ending new lines/spaces from pasted content
4 participants