Skip to content

[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

Merged
merged 24 commits into from
Jun 17, 2025

Conversation

htcfreek
Copy link
Collaborator

@htcfreek htcfreek commented Feb 25, 2025

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.

image

image

PR Checklist

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

Local test build.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

@htcfreek htcfreek self-assigned this Feb 25, 2025

This comment has been minimized.

This comment has been minimized.

@htcfreek htcfreek added the Status-Blocked We can't make progress due to a dependency or issue label Feb 25, 2025
@htcfreek htcfreek marked this pull request as ready for review March 1, 2025 00:04
@htcfreek htcfreek marked this pull request as draft March 1, 2025 22:04
@htcfreek htcfreek added the Product-Registry Preview Refers to the Registry Preview PowerToy label Mar 2, 2025
@randyrants
Copy link
Collaborator

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.

@htcfreek
Copy link
Collaborator Author

@randyrants
That makes so much sense.

This comment has been minimized.

@htcfreek htcfreek added the Idea-Enhancement New feature or request on an existing product label Mar 26, 2025
@htcfreek htcfreek requested a review from Copilot April 6, 2025 15:21
Copy link
Contributor

@Copilot Copilot AI left a 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.

@htcfreek htcfreek added Needs-Review This Pull Request awaits the review of a maintainer. and removed Status-Blocked We can't make progress due to a dependency or issue labels Apr 21, 2025
@htcfreek htcfreek marked this pull request as ready for review April 21, 2025 17:45
@htcfreek htcfreek requested a review from zhaopy536 April 21, 2025 17:47
@zhaopy536 zhaopy536 added the Don't merge - Hold for release Hold off on merging this PR, even if it's approved. label Apr 25, 2025
@htcfreek
Copy link
Collaborator Author

@crutkas
Can we have a "in for .92" label for this?

@crutkas
Copy link
Member

crutkas commented May 15, 2025

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@crutkas crutkas added In for .92 and removed Don't merge - Hold for release Hold off on merging this PR, even if it's approved. labels May 15, 2025
@yeelam-gordon
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yeelam-gordon
Copy link
Contributor

@htcfreek, I just rebased your branch with main. There are many conflicts with the change probably you made 2 months ago.
Please see the rebase itself make sense.

Copy link
Contributor

@yeelam-gordon yeelam-gordon left a 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 ;-)

@yeelam-gordon yeelam-gordon merged commit b2d7182 into microsoft:main Jun 17, 2025
10 checks passed
@htcfreek htcfreek deleted the PT_RegPrevSave branch June 17, 2025 05:09
yeelam-gordon added a commit that referenced this pull request Jun 20, 2025
<!-- 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.


![image](https://github.com/user-attachments/assets/9045446e-e9a3-4b81-8aa0-515b0821a969)


![image](https://github.com/user-attachments/assets/0888fbd2-851b-4101-a177-be9a3675b5ae)


<!-- 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Idea-Enhancement New feature or request on an existing product In for .92 Needs-Review This Pull Request awaits the review of a maintainer. Product-Registry Preview Refers to the Registry Preview PowerToy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants