Display Key Mappings in Settings #2314
Conversation
traies
force-pushed
the
user/udit3333/Apply-remap-on-ui
branch
from
6a3020f to
4686458
Compare
There was a problem hiding this comment.
LGTM! Just left a few comments on some clarifications. I also noticed that there are some logo files added which were previously removed in #2186 . Any reason for adding those back in?
src/core/Microsoft.PowerToys.Settings.UI/Microsoft.PowerToys.Settings.UI.csproj
Outdated
Show resolved
Hide resolved
src/core/Microsoft.PowerToys.Settings.UI/ViewModels/KeyboardManagerViewModel.cs
Outdated
Show resolved
Hide resolved
| if (SettingsUtils.SettingsExists()) | ||
| { | ||
| generalSettings = SettingsUtils.GetSettings<GeneralSettings>(string.Empty); | ||
| } | ||
| else | ||
| { | ||
| generalSettings = new GeneralSettings(); | ||
| SettingsUtils.SaveSettings(generalSettings.ToJsonString(), string.Empty); | ||
| } |
There was a problem hiding this comment.
Why are we initializing the general settings here? Shouldn't be done in the view model for the general page? Or is that the new approach that all powertoys are using?
There was a problem hiding this comment.
You are right, we should load the GeneralSettings file only once in memory and share it between the different pages.
|
Also we should create an issue to track the input language change does not reflect in SettingsV2 until the control is reloaded. This would probably be a v1 thing or maybe even future, but it's good to keep track of it. |
Summary of the Pull Request
References
PR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed