[General]Support language change #34971
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
🔥🔥🔥🔥🔥🔥 |
|
stefansjfw
requested review from
crutkas,
DHowett and
ethanfangg
as code owners
|
Ooooooo it past CI. Is this ready? |
jaimecbernardo
added
the
Needs-Review
Ready for review :) |
jaimecbernardo
changed the title
To prevent dependency hell. One day we'll refactor Settings.UI.Library and everything will be easier :)
I don't think so, it should be clear that it's a language change only. We can easily add it in the future if we see that needs to be clarified |
There was a problem hiding this comment.
check-spelling found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
stefansjfw
force-pushed
the
stefan/lang_override
branch
from
c230cbd
to
3e2b9fe
Compare
src/common/utils/resources.h
Outdated
|
|
||
| inline std::wstring get_resource_string_language_override(UINT resource_id, HINSTANCE instance) | ||
| { | ||
| std::wstring language = LanguageHelpers::load_language(); |
There was a problem hiding this comment.
If this is supposed to require a PowerToys restart, shouldn't we cache this result somehow? Thinking it's not good to do a file read and json parse on each time we want to get a resource.
There was a problem hiding this comment.
should this block on this or can it be addressed in new PR?
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| @@ -58,3 +58,12 @@ $imageResizerContextMenuAppManifestReadFileLocation = $imageResizerContextMenuAp | |||
| $imageResizerContextMenuAppManifest.Package.Identity.Version = $versionNumber + '.0' | |||
| Write-Host "ImageResizerContextMenu version" $imageResizerContextMenuAppManifest.Package.Identity.Version | |||
| $imageResizerContextMenuAppManifest.Save($imageResizerContextMenuAppManifestWriteFileLocation); | |||
|
|
|||
| # Set FileLocksmithContextMenu package version in AppManifest.xml | |||
| $fileLocksmithContextMenuAppManifestWriteFileLocation = $PSScriptRoot + '/../src/modules/FileLocksmith/FileLocksmithContextMenu/AppxManifest.xml'; | |||
There was a problem hiding this comment.
this seems unrelated to the change described herein - should it be a separate PR?
There was a problem hiding this comment.
Whatever regression happened here seems to have been caused by our changes, which is weird.
Anyway, this is something done for the other tier 1 context menus and not this one.
Since it was only bringing issues with this PR, we're just including it here.
There was a problem hiding this comment.
Basically, when testing this PR and tried to change the language, File Locksmith wouldn't appear in Windows 11 Tier 1 context menu, while still appearing on main. That's why we understood that we were missing those instructions. Makes sense @DHowett ? 😄
Summary of the Pull Request
PR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed