Skip to content

Feature: Added run as admin toggle to shortcut properties #10833

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

Merged
merged 38 commits into from
Jan 15, 2023

Conversation

hecksmosis
Copy link
Contributor

@hecksmosis hecksmosis commented Dec 23, 2022

Resolved / Related Issues
Items resolved / related issues by this PR.

Validation
How did you test these changes?

  • Built and ran the app
  • Tested the changes for accessibility

@hecksmosis
Copy link
Contributor Author

I am making this a draft because the refresh function is quite suboptimal (it refreshes everything on the page), I'm going to look for a solution but help would be appreciated.

@QuaintMako
Copy link
Contributor

Could you fix the formatting that was modified by accident but has nothing to do with the PR ? Like in src/Files.App/Views/Pages/PropertiesGeneral.xaml so it'd be easier to review what really needs to be? Thanks in advance!

@hecksmosis
Copy link
Contributor Author

Yes, idk why but my formatting turns strange when I push changes to github

@gave92
Copy link
Member

gave92 commented Dec 26, 2022

This should not be shown/enabled for folder shotcuts. Only for shortcuts pointing to an executable.

@hecksmosis
Copy link
Contributor Author

hecksmosis commented Dec 26, 2022

It is already disabled in folder shortcuts, resembling the read-only attribute, which also is disabled for folders, etc.

@gave92
Copy link
Member

gave92 commented Dec 26, 2022

I see it enabled for shortcuts to folder:

And enabled for shortcuts to txt:

@hecksmosis
Copy link
Contributor Author

I'll fix it tomorrow

@hecksmosis
Copy link
Contributor Author

hecksmosis commented Dec 27, 2022

Should I try to change the refresh function? (As it is not in the main page any more, I find it unlikely that many people will change this repeatedly, and therefore it will not have a significant effect on performance)

@yaira2
Copy link
Member

yaira2 commented Dec 28, 2022

Turning the toggle off sometimes requires multiple clicks before it works.

@hecksmosis
Copy link
Contributor Author

It's weird, I cannot reproduce this consistently.

@hecksmosis
Copy link
Contributor Author

@yaira2 This looks like an issue with the xaml component, as it also happens in parts of the settings page with the same component.

@yaira2
Copy link
Member

yaira2 commented Dec 31, 2022

Which setting?

@hecksmosis
Copy link
Contributor Author

The ones that use a ToggleSwitch occasionally have this issue

@hecksmosis
Copy link
Contributor Author

It looks like a known issue:
microsoft/microsoft-ui-xaml#3796

@yaira2 yaira2 changed the title Feature: Run as admin for shortcuts Feature: Added run as admin toggle to shortcut properties Jan 4, 2023
yaira2
yaira2 previously approved these changes Jan 11, 2023
Copy link
Member

@yaira2 yaira2 left a comment

Choose a reason for hiding this comment

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

LGTM

QuaintMako
QuaintMako previously approved these changes Jan 11, 2023
@hecksmosis
Copy link
Contributor Author

Is this ok then?

@hecksmosis hecksmosis dismissed stale reviews from QuaintMako and yaira2 via d0f6f26 January 15, 2023 17:17
Copy link
Member

@yaira2 yaira2 left a comment

Choose a reason for hiding this comment

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

LGTM

@yaira2 yaira2 added ready to merge Pull requests that are approved and ready to merge and removed needs - code review labels Jan 15, 2023
@yaira2 yaira2 merged commit 7d3c2f6 into files-community:main Jan 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Pull requests that are approved and ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: Add an option to the properties window to run shortcuts as admin
4 participants