Skip to content
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

Add functionality to settings Open JSON file entry to open the settings.json parent folder in Explorer #12382

Open
darkred opened this issue Feb 4, 2022 · 9 comments
Labels
Area-Settings UI Anything specific to the SUI good first issue This is a fix that might be easier for someone to do as a first contribution Help Wanted We encourage anyone to jump in on these. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal.
Milestone

Comments

@darkred
Copy link

darkred commented Feb 4, 2022

Description of the new feature/enhancement

(this is a follow up to this issue #1460)

I have Windows Terminal latest stable, v1.12.10334.0 , on win11.

Currently, in the dropdown menu | 'Settings', there's an Open JSON file entry, clicking which opens the settings.json file only if the user has associated the .json file type with an installed editor, otherwise it shows the "Do you want to open this file?" windows dialog .

screen capture:

If you close the dialog, in order to manage to open the settings.json file manually, you'll have to look in the project wiki FAQ to find its location/path - there's no related info shown inside the app.

So, my suggestion is add functionality to "Open JSON file" settings entry to open the settings.json parent folder in Explorer.

Proposed technical implementation details (optional)

In details, my suggestion is, when clicking the Open JSON file entry, instead of attempting to open settings.json with the associated editor, to display a menu with two entries:

  • Open settings.json with associated editor
  • Open parent folder of settings.json in Explorer (in order to be able to open it manually).

 
Or, alternatively/more simply, keep the current behavior (=attempt to open the file with the associated editor when leftclicking the "Open JSON file" entry), but when rightclicking it, to open its parent folder in file explorer (and update the entry tooltip respectively).

@darkred darkred added the Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. label Feb 4, 2022
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Feb 4, 2022
@darkred darkred changed the title Add functionality to "Open JSON file" settings entry to open the settings.json parent folder Add functionality to settings "Open JSON file" entry to open the settings.json parent folder in Explorer Feb 5, 2022
@darkred darkred changed the title Add functionality to settings "Open JSON file" entry to open the settings.json parent folder in Explorer Add functionality to settings Open JSON file entry to open the settings.json parent folder in Explorer Feb 6, 2022
@zadjii-msft
Copy link
Member

This is a clever idea.

Other ideas to build on this:

We change the button at the bottom of the nav view to a "More options" button with a flyout, and in there, stick:

@zadjii-msft zadjii-msft added Area-Settings UI Anything specific to the SUI Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. and removed Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. labels Feb 8, 2022
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Feb 8, 2022
@zadjii-msft zadjii-msft added this to the 22H2 milestone Feb 8, 2022
@zadjii-msft zadjii-msft removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Feb 17, 2022
@zadjii-msft zadjii-msft modified the milestones: 22H2, Backlog May 5, 2022
@zadjii-msft zadjii-msft added Help Wanted We encourage anyone to jump in on these. good first issue This is a fix that might be easier for someone to do as a first contribution labels May 5, 2022
@Swinkid
Copy link
Contributor

Swinkid commented Mar 16, 2023

@zadjii-msft I'll give this a punt if thats okay. Would need some pointers, particularly surrounding how the UI works!

@zadjii-msft
Copy link
Member

@Swinkid sorry I lost that in my inbox! Okay, here's some thoughts:

Note

Walkthrough

The "Open JSON File" button is defined here:

<muxc:NavigationView.FooterMenuItems>
<!-- The OpenJson item needs both Tapped and KeyDown handler -->
<muxc:NavigationViewItem x:Name="OpenJsonNavItem"
x:Uid="Nav_OpenJSON"
KeyDown="OpenJsonKeyDown"
Tapped="OpenJsonTapped">
<muxc:NavigationViewItem.Icon>
<FontIcon Glyph="&#xE713;" />
</muxc:NavigationViewItem.Icon>
</muxc:NavigationViewItem>
</muxc:NavigationView.FooterMenuItems>

I think it'd be easy to add a context menu flyout to it. Probably something like

                <NavigationViewItem.ContextFlyout>
                    <MenuFlyoutItem></MenuFlyoutItem>
                </NavigationViewItem.ContextFlyout>

To set the .ContextFlyout on it

It might be harder to change that button to like, a split button. But OP did say it was okay to implement this as a right-click context menu.

As far as what the button should actually do - MainPage::OpenJsonTapped currently just bubbles a SettingsTarget to the handler in fire_and_forget TerminalPage::_LaunchSettings.

So I'd:

  • add a new SettingsTarget for SettingsDirectory to ActionArgs.idl
  • Add a button handler to the MenuFlyoutItem in MainPage that just calls _OpenJsonHandlers(nullptr, SettingsTarget::SettingsDirectory);
  • add a case in TerminalPage::_LaunchSettings to open the directory for the settings, rather than the file itself.

@zadjii-msft zadjii-msft moved this to Walkthrough in issue in Terminal Walkthroughs Apr 28, 2023
AbdullahAlmanei added a commit to AbdullahAlmanei/terminal that referenced this issue May 24, 2023
@AbdullahAlmanei
Copy link

Created a PR for this issue! Take a look here: #15417
I couldn't set the Tooltip and the text through the resources for some reason. (It kept crashing whenever I tried linking the resources to the flyout menu using x:Uid) I might have been missing something simple, but I am not sure what exactly.

@AbdullahAlmanei
Copy link

@zadjii-msft for #15417, would setting the text through resources be necessary? I've still not worked out why it's not working.

@zadjii-msft
Copy link
Member

The resources kinda are, actually. They're used for localization purposes, so only the .resw files get translated, and the .xaml files basically pick the correct translation up automatically.

That's pretty much entirely keyed off the x:Uid property in XAML. (I'll leave comments on the PR ☺️)

@marcelwgn
Copy link
Contributor

@AbdullahAlmanei and @Swinkid are any of you working on this or is this up for grabs?

@Swinkid
Copy link
Contributor

Swinkid commented Mar 21, 2024

@AbdullahAlmanei and @Swinkid are any of you working on this or is this up for grabs?

Go ahead!

@Ismith507
Copy link

Hello, I noticed that this issue has a stale PR(#15417) that is almost a year old. In the PR it seems like a diferent solution was proposed and still needs to be done. Should the solution be based on what was discussed in #15417, or is the proposed solution in this thread still relevant? I would like to submit my own PR and want to know what direction I should be going in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Settings UI Anything specific to the SUI good first issue This is a fix that might be easier for someone to do as a first contribution Help Wanted We encourage anyone to jump in on these. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal.
Projects
Status: Walkthrough in issue
Development

No branches or pull requests

6 participants