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

[Registry Preview] Better preview in data grid like in Regedit #28488

Merged

Conversation

dillydylann
Copy link
Contributor

@dillydylann dillydylann commented Sep 10, 2023

Summary of the Pull Request

This PR updates the data grid in Registry Preview to properly show the data values shown in Registry Editor.

PR Checklist

  • Closes: [Regostry Preview] Convert data (binary) values #25118
  • Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
  • Tests: Added/updated and all pass
  • Localization: All end user facing strings can be localized
  • Dev docs: No changes needed
  • New binaries: No changes made
  • Documentation updated: No changes needed

Detailed Description of the Pull Request / Additional comments

Left: Registry Preview | Right: Registry Editor

image

@dillydylann dillydylann marked this pull request as ready for review September 10, 2023 23:45
@randyrants
Copy link
Collaborator

Hah: when I saw the screenshots, I thought the goal was to replace the WinUI 3 grid with a LIstView, which made me very conflicted.

In terms of testing this, please revisit all the resolved bugs on the program: there are lots of little things reported by people that wanted the app to be a "creator" rather than an "editor" which uncovered to issues with odd things. Like trailing comments on data lines or data that crosses over two lines but with added white space. Made the "parsing" routine a tad larger and convoluted, sort of like using bubble gum to plug holes.

I also assume that multiple languages were included in the testing, in addition to the emoji? When asked "why aren't you showing text for SZ_MULTI" I replied with NYI and ran away from the question! :)

@randyrants randyrants added the Product-Registry Preview Refers to the Registry Preview PowerToy label Sep 13, 2023
@htcfreek
Copy link
Collaborator

Would it make sense to support Reg_Multi_Sz multi-line preview in a read-only content Dialog?

@jaimecbernardo
Copy link
Collaborator

Hi @randyrants , do you feel the recent changes addressed your feedback? 😉

Copy link
Collaborator

@randyrants randyrants left a comment

Choose a reason for hiding this comment

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

LG2M! We'll know more as people start to run their files through it - that's where I found the majority of bugs with stuff like this.

@randyrants randyrants merged commit 1de6c7d into microsoft:main Sep 21, 2023
7 checks passed
@dillydylann
Copy link
Contributor Author

Would it make sense to support Reg_Multi_Sz multi-line preview in a read-only content Dialog?

@htcfreek What about showing it in a flyout or tooltip?

@htcfreek
Copy link
Collaborator

Would it make sense to support Reg_Multi_Sz multi-line preview in a read-only content Dialog?

@htcfreek What about showing it in a flyout or tooltip?

@dillydylann
Sounds good. I would prefer zhe flyout. Maybe we could add a glass in the datagrid that opens the flyout.

(Should I open a new issues?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Product-Registry Preview Refers to the Registry Preview PowerToy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants