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

Launch/Open/GoTo: Userfacing strings #32519

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

Jay-o-Way
Copy link
Collaborator

@Jay-o-Way Jay-o-Way commented Apr 18, 2024

Summary of the Pull Request

Split from #32351
Closely linked to #32514 and #32516 and #32450.

PR Checklist

  • Closes: "Open", "Launch", "Go to"... #30685
  • Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
  • Localization: All end user facing strings can be localized
  • Dev docs: Added/updated
  • Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Validation Steps Performed

Build

@Jay-o-Way Jay-o-Way requested a review from crutkas April 18, 2024 13:20
@Jay-o-Way Jay-o-Way added this to the PowerToys 0.81 milestone Apr 18, 2024
@Jay-o-Way Jay-o-Way marked this pull request as ready for review April 18, 2024 13:21
value : The object must be serialized into a byte array
: using a System.ComponentModel.TypeConverter
: and then encoded with base64 encoding.
-->
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why add all these comments?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's all done auto by Visual Studio.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

P.S. Using resW versus resX is a different and larger topic

@@ -89,7 +148,7 @@
<value>This setting has been disabled by your administrator.</value>
</data>
<data name="STARTUP_DISABLED_BY_USER" xml:space="preserve">
<value>This setting has been disabled manually via &lt;a href=&quot;https://ms_settings_startupapps&quot; target=&quot;_blank&quot;&gt;Startup Settings&lt;/a&gt;.</value>
<value>This setting has been disabled manually via &lt;a href="https://ms_settings_startupapps" target="_blank"&gt;Startup Settings&lt;/a&gt;.</value>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this change working? Looks like unneeded change with the context of the PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's all done auto by Visual Studio.

Copy link
Member

Choose a reason for hiding this comment

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

this needs to be validated it works if it was changed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@jaimecbernardo @crutkas If I interpret this result correctly, this string isn't even used anywhere. Does that answer your questions? 😆
image

Copy link
Member

Choose a reason for hiding this comment

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

As it has a href, this is legacy then from the react style app that never got refactored out when that subsystem was removed

@jaimecbernardo
Copy link
Collaborator

Thanks for opening the PR :) Added some comments just from a quick glance.

@Jay-o-Way
Copy link
Collaborator Author

Jay-o-Way commented Apr 18, 2024

(off-topic) @ethanfangg I think the merge queue would be ideal for these 3 PRs + #32450

@Jay-o-Way Jay-o-Way changed the title Launch open go: Userfacing strings Launch/Open/GoTo: Userfacing strings Apr 22, 2024
@Jay-o-Way
Copy link
Collaborator Author

Is there anything blocking this?

@jaimecbernardo
Copy link
Collaborator

In general looks good, but will revisit this series or PRs on .82 to not interfere with localization for Build right now, since it does change quite some strings. Hope this makes sense.

@Jay-o-Way
Copy link
Collaborator Author

Hope this makes sense.

Well, I don't have the knowledge that you have. But if you say so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants