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

Feature: Show error when a shortcut can't be created #11013

Conversation

ferrariofilippo
Copy link
Contributor

@ferrariofilippo ferrariofilippo commented Jan 14, 2023

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

Details

  • Fixed a bug with shortcut's names created from modal (lines 82-86 CreateShortcutDialogViewModel.cs
  • Added Can't create shortcut modal (I'm not sure if it should be displayed when File/Folder properties are edited)

Validation
How did you test these changes?

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

Screenshots (optional)
1

@yaira2 yaira2 added the changes requested Changes are needed for this pull request label Jan 15, 2023
@ferrariofilippo
Copy link
Contributor Author

I'm not sure if it should be displayed when File/Folder properties are edited

@yaira2 what about this?

@yaira2
Copy link
Member

yaira2 commented Jan 15, 2023

What do you mean by this?

@ferrariofilippo
Copy link
Contributor Author

At the moment, when the user opens Properties of a shortcut, he/she can edit its attributes. When this happens, Files tries to update the shortcut. If this is placed in a protected directory (such as C:\) nothing happens and the user doesn't get any error.

What I'm asking is whether the Create on desktop modal should be displayed in this specific case.
If the answer is no, should we inform the user that he/she needs to run Files as admin to successfully edit the shortcut?

@yaira2
Copy link
Member

yaira2 commented Jan 19, 2023

I didn't realize it can be worked around if the user runs as administrator. We should update the message accordingly. Could try something like this "This action requires administrator privileges", "Please run Files as administrator to create this shortcut".

@ferrariofilippo
Copy link
Contributor Author

ferrariofilippo commented Jan 19, 2023

I didn't realize it can be worked around if the user runs as administrator. We should update the message accordingly.

I think we should give the user the ability to create it on Desktop: if, for any reason , he can't run Files as admin, he should still be able to create that shortcut.

The modal might be like:
Title: This action requires administrator privileges
Message: Please, run Files as administrator or create it on Desktop

@ferrariofilippo
Copy link
Contributor Author

ferrariofilippo commented Jan 19, 2023

I have to inform you that I have temporarily removed the binding from ItemDestinationPath. Now this property is being updated manually. I did this because I noticed that the Binding was not working anymore. In fact, if you try to create a shortcut from the modal (even in stable version), you'll see that it doesn't work (actually sometimes it works).

I know this is not optimal and I'll work to reintroduce the binding
Edit. Done

@yaira2 yaira2 changed the title Feature: "Can't create shortcut" modal Feature: Error is displayed when a shortcut can't be created Jan 24, 2023
@yaira2 yaira2 added needs - code review and removed changes requested Changes are needed for this pull request labels Jan 24, 2023
@yaira2 yaira2 changed the title Feature: Error is displayed when a shortcut can't be created Feature: Show error when a shortcut can't be created Jan 24, 2023
yaira2
yaira2 previously approved these changes Feb 1, 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

@yaira2 yaira2 added ready to merge Pull requests that are approved and ready to merge and removed needs - code review labels Feb 1, 2023
@yaira2 yaira2 merged commit 0efb5aa into files-community:main Feb 1, 2023
@ferrariofilippo ferrariofilippo deleted the Feature_Cannot_Create_Shortcut_Dialog branch February 1, 2023 22:48
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: "Can't create shortcut" modal
3 participants