-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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
[File Explorer] Added an overall toggle switch (#23723) #33475
base: main
Are you sure you want to change the base?
Changes from 15 commits
7e4e12b
79919c9
2b513b6
9dabee3
d6bd83b
b65f633
a40d5c5
cd3172c
40707c4
75e8ed9
43074da
877d12c
e5798ee
9e85b63
b35df52
f197ee3
4f15ce6
7d1f213
b905634
e3e9583
1470338
4c42c90
c9174f9
7a4052d
779214e
115d3e8
9c83638
b31bcab
7f6ab6f
893e5de
34381ad
dc1746a
304ca76
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,11 @@ | ||
<?xml version="1.0" encoding="utf-8"?> | ||
<!-- Copyright (c) Microsoft Corporation. | ||
Licensed under the MIT License. --> | ||
<policyDefinitions xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" revision="1.10" schemaVersion="1.0" xmlns="http://schemas.microsoft.com/GroupPolicy/2006/07/PolicyDefinitions"> | ||
<policyDefinitions xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" revision="1.11" schemaVersion="1.0" xmlns="http://schemas.microsoft.com/GroupPolicy/2006/07/PolicyDefinitions"> | ||
<policyNamespaces> | ||
<target prefix="powertoys" namespace="Microsoft.Policies.PowerToys" /> | ||
</policyNamespaces> | ||
<resources minRequiredRevision="1.10"/><!-- Last changed with PowerToys v0.81.1 --> | ||
<resources minRequiredRevision="1.11"/><!-- Last changed with PowerToys v0.82.0 --> | ||
<supportedOn> | ||
<definitions> | ||
<definition name="SUPPORTED_POWERTOYS_0_64_0" displayName="$(string.SUPPORTED_POWERTOYS_0_64_0)"/> | ||
|
@@ -19,6 +19,7 @@ | |
<definition name="SUPPORTED_POWERTOYS_0_78_0" displayName="$(string.SUPPORTED_POWERTOYS_0_78_0)"/> | ||
<definition name="SUPPORTED_POWERTOYS_0_81_0" displayName="$(string.SUPPORTED_POWERTOYS_0_81_0)"/> | ||
<definition name="SUPPORTED_POWERTOYS_0_81_1" displayName="$(string.SUPPORTED_POWERTOYS_0_81_1)"/> | ||
<definition name="SUPPORTED_POWERTOYS_0_82_0" displayName="$(string.SUPPORTED_POWERTOYS_0_82_0)"/> | ||
</definitions> | ||
</supportedOn> | ||
<categories> | ||
|
@@ -136,6 +137,16 @@ | |
<decimal value="0" /> | ||
</disabledValue> | ||
</policy> | ||
<policy name="ConfigureEnabledUtilityFileExplorerPreview" class="Both" displayName="$(string.ConfigureEnabledUtilityFileExplorerPreview)" explainText="$(string.ConfigureEnabledUtilityDescription)" key="Software\Policies\PowerToys" valueName="ConfigureEnabledUtilityFileExplorerPreview"> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done (line 49). |
||
<parentCategory ref="PowerToys" /> | ||
<supportedOn ref="SUPPORTED_POWERTOYS_0_82_0" /> | ||
<enabledValue> | ||
<decimal value="1" /> | ||
</enabledValue> | ||
<disabledValue> | ||
<decimal value="0" /> | ||
</disabledValue> | ||
</policy> | ||
<policy name="ConfigureEnabledUtilityFileExplorerSVGPreview" class="Both" displayName="$(string.ConfigureEnabledUtilityFileExplorerSVGPreview)" explainText="$(string.ConfigureEnabledUtilityDescription)" key="Software\Policies\PowerToys" valueName="ConfigureEnabledUtilityFileExplorerSVGPreview"> | ||
<parentCategory ref="PowerToys" /> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could move all this File Explorer preview GPOs into a new subcategory "File Previewers". That makes it easier to find the GPOs for the single previewers. What do you think? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was thinking about that when I made the GPO. I think we should. It was hard to find the overall GPO even when I knew exactly what I was looking for. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But the overall gpo has to keep in the main folder. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
<supportedOn ref="SUPPORTED_POWERTOYS_0_64_0" /> | ||
|
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -166,6 +166,18 @@ void PowerPreviewModule::enable() | |||
} | ||||
|
||||
m_enabled = true; | ||||
|
||||
try | ||||
{ | ||||
PowerToysSettings::PowerToyValues settings = | ||||
PowerToysSettings::PowerToyValues::load_from_settings_file(PowerPreviewModule::get_key()); | ||||
|
||||
apply_settings(settings); | ||||
} | ||||
catch (std::exception const& e) | ||||
{ | ||||
Trace::InitSetErrorLoadingFile(e.what()); | ||||
} | ||||
} | ||||
|
||||
// Disable active preview handlers. | ||||
|
@@ -218,56 +230,59 @@ void PowerPreviewModule::apply_settings(const PowerToysSettings::PowerToyValues& | |||
{ | ||||
bool notifyShell = false; | ||||
|
||||
for (auto& fileExplorerModule : m_fileExplorerModules) | ||||
if (m_enabled) | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @htcfreek There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure. But the policy state won't be written into the settings file. You have to explicit query the policy values and evaluate them. Like here:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because you shouldn't be able to bypass a force-enabled add-on by turning off all extensions? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But what about force disabled? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you think of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, it does look good to me now. |
||||
{ | ||||
const auto toggle = settings.get_bool_value(fileExplorerModule.settingName); | ||||
const auto gpo_rule = fileExplorerModule.checkModuleGPOEnabledRuleFunction(); | ||||
const auto gpo_is_configured = gpo_rule == powertoys_gpo::gpo_rule_configured_enabled || gpo_rule == powertoys_gpo::gpo_rule_configured_disabled; | ||||
|
||||
if (gpo_rule == powertoys_gpo::gpo_rule_configured_unavailable) | ||||
{ | ||||
Logger::warn(L"Couldn't read the gpo rule for Power Preview module {}", fileExplorerModule.settingName); | ||||
} | ||||
if (gpo_rule == powertoys_gpo::gpo_rule_configured_wrong_value) | ||||
{ | ||||
Logger::warn(L"gpo rule for Power Preview module {} is set to an unknown value", fileExplorerModule.settingName); | ||||
} | ||||
|
||||
// Skip if no need to update | ||||
if (!toggle.has_value() && !gpo_is_configured) | ||||
{ | ||||
continue; | ||||
} | ||||
|
||||
bool module_new_state = false; | ||||
if (toggle.has_value()) | ||||
{ | ||||
module_new_state = *toggle; | ||||
} | ||||
if (gpo_is_configured) | ||||
{ | ||||
// gpo rule overrides settings state | ||||
module_new_state = gpo_rule == powertoys_gpo::gpo_rule_configured_enabled; | ||||
} | ||||
|
||||
// Skip if no need to update | ||||
if (module_new_state == fileExplorerModule.registryChanges.isApplied()) | ||||
{ | ||||
continue; | ||||
} | ||||
|
||||
// (Un)Apply registry changes depending on the new setting value | ||||
const bool updated = module_new_state ? fileExplorerModule.registryChanges.apply() : fileExplorerModule.registryChanges.unApply(); | ||||
|
||||
if (updated) | ||||
{ | ||||
notifyShell = true; | ||||
Trace::PowerPreviewSettingsUpdated(fileExplorerModule.settingName.c_str(), !*toggle, *toggle, true); | ||||
} | ||||
else | ||||
for (auto& fileExplorerModule : m_fileExplorerModules) | ||||
{ | ||||
Logger::error(L"Couldn't {} file explorer module {} during apply_settings", *toggle ? L"enable " : L"disable", fileExplorerModule.settingName); | ||||
Trace::PowerPreviewSettingsUpdateFailed(fileExplorerModule.settingName.c_str(), !*toggle, *toggle, true); | ||||
const auto toggle = settings.get_bool_value(fileExplorerModule.settingName); | ||||
const auto gpo_rule = fileExplorerModule.checkModuleGPOEnabledRuleFunction(); | ||||
const auto gpo_is_configured = gpo_rule == powertoys_gpo::gpo_rule_configured_enabled || gpo_rule == powertoys_gpo::gpo_rule_configured_disabled; | ||||
|
||||
if (gpo_rule == powertoys_gpo::gpo_rule_configured_unavailable) | ||||
{ | ||||
Logger::warn(L"Couldn't read the gpo rule for Power Preview module {}", fileExplorerModule.settingName); | ||||
} | ||||
if (gpo_rule == powertoys_gpo::gpo_rule_configured_wrong_value) | ||||
{ | ||||
Logger::warn(L"gpo rule for Power Preview module {} is set to an unknown value", fileExplorerModule.settingName); | ||||
} | ||||
|
||||
// Skip if no need to update | ||||
if (!toggle.has_value() && !gpo_is_configured) | ||||
{ | ||||
continue; | ||||
} | ||||
|
||||
bool module_new_state = false; | ||||
if (toggle.has_value()) | ||||
{ | ||||
module_new_state = *toggle; | ||||
} | ||||
if (gpo_is_configured) | ||||
{ | ||||
// gpo rule overrides settings state | ||||
module_new_state = gpo_rule == powertoys_gpo::gpo_rule_configured_enabled; | ||||
} | ||||
|
||||
// Skip if no need to update | ||||
if (module_new_state == fileExplorerModule.registryChanges.isApplied()) | ||||
{ | ||||
continue; | ||||
} | ||||
|
||||
// (Un)Apply registry changes depending on the new setting value | ||||
const bool updated = module_new_state ? fileExplorerModule.registryChanges.apply() : fileExplorerModule.registryChanges.unApply(); | ||||
|
||||
if (updated) | ||||
{ | ||||
notifyShell = true; | ||||
Trace::PowerPreviewSettingsUpdated(fileExplorerModule.settingName.c_str(), !*toggle, *toggle, true); | ||||
} | ||||
else | ||||
{ | ||||
Logger::error(L"Couldn't {} file explorer module {} during apply_settings", *toggle ? L"enable " : L"disable", fileExplorerModule.settingName); | ||||
Trace::PowerPreviewSettingsUpdateFailed(fileExplorerModule.settingName.c_str(), !*toggle, *toggle, true); | ||||
} | ||||
} | ||||
} | ||||
if (notifyShell) | ||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -56,7 +56,7 @@ public bool ImageResizer | |
|
||
private bool fileExplorerPreview = true; | ||
|
||
[JsonPropertyName("File Explorer Preview")] | ||
[JsonPropertyName("File Explorer")] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this change breake existing settings that users made? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The property |
||
public bool PowerPreview | ||
{ | ||
get => fileExplorerPreview; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,7 +13,20 @@ | |
<controls:SettingsPageControl x:Uid="FileExplorerPreview" ModuleImageSource="ms-appx:///Assets/Settings/Modules/FileExplorerPreview.png"> | ||
<controls:SettingsPageControl.ModuleContent> | ||
<StackPanel ChildrenTransitions="{StaticResource SettingsCardsAnimations}" Orientation="Vertical"> | ||
<controls:SettingsGroup x:Uid="FileExplorerPreview_PreviewPane"> | ||
<tkcontrols:SettingsCard | ||
x:Uid="FileExplorerPreview_EnableToggleControl_HeaderText" | ||
HeaderIcon="{ui:BitmapIcon Source=/Assets/Settings/Icons/FileExplorerPreview.png}" | ||
IsEnabled="{x:Bind ViewModel.IsEnabledGpoConfigured, Mode=OneWay, Converter={StaticResource BoolNegationConverter}}"> | ||
<ToggleSwitch x:Uid="ToggleSwitch" IsOn="{x:Bind ViewModel.IsEnabled, Mode=TwoWay}" /> | ||
</tkcontrols:SettingsCard> | ||
<InfoBar | ||
x:Uid="GPO_SettingIsManaged" | ||
IsClosable="False" | ||
IsOpen="{x:Bind ViewModel.IsEnabledGpoConfigured, Mode=OneWay}" | ||
IsTabStop="{x:Bind ViewModel.IsEnabledGpoConfigured, Mode=OneWay}" | ||
Severity="Informational" /> | ||
|
||
<controls:SettingsGroup x:Uid="FileExplorerPreview_PreviewPane" IsEnabled="{x:Bind ViewModel.IsEnabled, Mode=OneWay}"> | ||
<InfoBar | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please hide all this info bars that are not related to the overall Module enabled GPO when the GPO is disabled. (We make it on the other pages the same.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The incompatibility warnings and the reboot required notice should be hidden when the GPO is set to Disabled? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or should I hide them whenever the switch is off regardless of GPO setting? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
yes.
yes You can implement a view model property "showInfobars" that is false if gpo is disabled or toggle is disabled (overall: if toggle is not enabled). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just used There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If it works as expected, yes. I didn't thought about this solution yet. |
||
x:Uid="FileExplorerPreview_PreviewHandlerOutlookIncompatibility" | ||
IsClosable="False" | ||
|
@@ -147,7 +160,7 @@ | |
</tkcontrols:SettingsCard> | ||
</controls:SettingsGroup> | ||
|
||
<controls:SettingsGroup x:Uid="FileExplorerPreview_IconThumbnail_GroupSettings"> | ||
<controls:SettingsGroup x:Uid="FileExplorerPreview_IconThumbnail_GroupSettings" IsEnabled="{x:Bind ViewModel.IsEnabled, Mode=OneWay}"> | ||
<InfoBar | ||
x:Uid="FileExplorerPreview_RebootRequired" | ||
IsClosable="False" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will be 0.83.0 . Please update all occurrences.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.