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

File Actions menu #31220

Open
wants to merge 168 commits into
base: main
Choose a base branch
from
Open

File Actions menu #31220

wants to merge 168 commits into from

Conversation

Aaron-Junker
Copy link
Collaborator

@Aaron-Junker Aaron-Junker commented Feb 1, 2024

Summary of the Pull Request

Adding a new menu invokable by selecting files in explorer and pressing a shortcut set in the settings.

Screenshots

image
image
image
image
image
image
image

PR Checklist

Detailed Description of the Pull Request / Additional comments

Things left to do:

  • Add installer
  • Add icon
  • DPI Awareness

Validation Steps Performed

</ui:MenuItem.Icon>
</ui:MenuItem>
<Separator/>
<ui:MenuItem Header="Copy path of files sperated by">

Check failure

Code scanning / check-spelling

Unrecognized Spelling

[sperated](#security-tab) is not a recognized word. \(unrecognized-spelling\)
@Aaron-Junker
Copy link
Collaborator Author

@niels9001 Do you have an idea why the contextmenu doesn't take the WPFUI style?

Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

check-spelling found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

This comment has been minimized.

@stefansjfw
Copy link
Collaborator

Thanks for the fix Aaron, I'll give it a try today and do another round of review

@htcfreek
Copy link
Collaborator

Is this planned to get in for 0.88 release?

@stefansjfw stefansjfw requested a review from a team as a code owner December 23, 2024 10:52
Copy link
Collaborator

@stefansjfw stefansjfw left a comment

Choose a reason for hiding this comment

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

I fixed the installer, now everything works.

I left few comments and issues to resolve.

Also, it would be good to put all of the FileActionsMenu DLLs to single out dir (I'd say WinUI3Apps if UI app is winui3) .. because now , some stuff goes to root output dir and some to WinUI3Apps dir. This ends up with helpers and intefraces DLLs being duplicated and installed to both which we don't want.

.github/actions/spell-check/expect.txt Outdated Show resolved Hide resolved
@@ -353,3 +353,6 @@ src/common/Telemetry/*.etl

# MSBuildCache
/MSBuildCacheLogs/

# Temp telemetry files.
*.etl
Copy link
Collaborator

Choose a reason for hiding this comment

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

why is this here? where do you see .etl files inside the repo? This shouldn't happen

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

When following the instructions in https://github.com/microsoft/PowerToys/tree/main/src/common/Telemetry to look at sent telemetry it produces an etl file.

Directory.Packages.props Outdated Show resolved Hide resolved
PowerToys.sln Outdated Show resolved Hide resolved
src/gpo/assets/PowerToys.admx Outdated Show resolved Hide resolved
tools/BugReportTool/BugReportTool/ProcessesList.cpp Outdated Show resolved Hide resolved
@Aaron-Junker
Copy link
Collaborator Author

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

This comment has been minimized.

@Aaron-Junker
Copy link
Collaborator Author

@stefansjfw All your comments are now adressed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs-Review This Pull Request awaits the review of a maintainer.
Projects
Status: 📑In Review
Status: No status
Development

Successfully merging this pull request may close these issues.

7 participants