-
Notifications
You must be signed in to change notification settings - Fork 7.1k
[RegPreview] Various improvements on how files are saved #37628
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
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
src/modules/registrypreview/RegistryPreviewUILib/RegistryPreviewMainPage.Events.cs
Show resolved
Hide resolved
Up to recently, the enablement of the Save button was the indicator that the file was dirty or not. With this new functionality in place, to add a * to the title of the toolbar, I would think it makes sense to move all settings of saveButton.IsEnabled into the UpdateUnsavedFileIndicator method, so no one forgets to set one versus the other in the future. JM2C. |
@randyrants |
src/modules/registrypreview/RegistryPreviewUILib/RegistryPreviewMainPage.Utilities.cs
Outdated
Show resolved
Hide resolved
src/modules/registrypreview/RegistryPreviewUILib/RegistryPreviewMainPage.Events.cs
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
src/modules/registrypreview/RegistryPreviewUILib/RegistryPreviewMainPage.Utilities.cs
Show resolved
Hide resolved
src/modules/registrypreview/RegistryPreviewUILib/RegistryPreviewMainPage.Events.cs
Show resolved
Hide resolved
@crutkas |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
6689097
to
abb36f3
Compare
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
@htcfreek, I just rebased your branch with main. There are many conflicts with the change probably you made 2 months ago. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try out locally, the functionality looks great ;-)
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? --> ## Summary of the Pull Request ### This PR implements various fixes and improvements into RegistryPreview for saving files: 1. Adds an unsaved file indicator in the title bar like in Notepad. (As indicator we show the * character before the file title.) 2. The save button behaves like a "save as" button, if the file does not exist on disk like in Notepad. (Without fix when running as non-admin you get an access denied error message.) 3. If the app gets closed without saving and the file does not exist on disk, then the user is now asked for the path. (Fixes crash on clicking save button while closing the app.) 4. Failed save actions are handled now correctly on dirty closing, opening files and all other actions that require saving the current state. They will stop the process. 5. A fix for an incorrect enabled state of the save button after opening, reloading and saving a file. 6. Reuse file name on save as button, if known. Otherwise use "new file" template name like in Notepad. 7. Fix an app crash if you click the window's close button a second time while the "Should save?" dialog is opened. 8. Added an reload dialog in case of unsaved changes.   <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist - [x] **Closes:** #36876, #36875 - [x] **Communication:** I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected - [x] **Tests:** Added/updated and all pass - [x] **Localization:** All end user facing strings can be localized - [ ] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx <!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here --> ## Detailed Description of the Pull Request / Additional comments <!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well --> ## Validation Steps Performed Local test build. --------- Co-authored-by: Gordon Lam (SH) <yeelam@microsoft.com>
Summary of the Pull Request
This PR implements various fixes and improvements into RegistryPreview for saving files:
PR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed
Local test build.