Skip to content

Commit caf8b3f

Browse files
ThanhNguyxnThanhNguyxn
committed
refactor: Address Copilot review suggestions
- Add 'sealed' modifier to IndexToBoolConverter class - Return DependencyProperty.UnsetValue instead of -1 in ConvertBack - Add Disabled visual state with 0.4 opacity for accessibility - Use x:Uid for RadioButton localization instead of hardcoded strings
1 parent 93cd0f6 commit caf8b3f

File tree

2 files changed

+28
-30
lines changed

2 files changed

+28
-30
lines changed

‎src/settings-ui/Settings.UI/Converters/IndexToBoolConverter.cs‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// See the LICENSE file in the project root for more information.
44

55
using System;
6+
using Microsoft.UI.Xaml;
67
using Microsoft.UI.Xaml.Data;
78

89
namespace Microsoft.PowerToys.Settings.UI.Converters
@@ -11,7 +12,7 @@ namespace Microsoft.PowerToys.Settings.UI.Converters
1112
/// Converts an integer index to a boolean value for use with RadioButton groups.
1213
/// The ConverterParameter specifies which index value should return true.
1314
/// </summary>
14-
public partial class IndexToBoolConverter : IValueConverter
15+
public sealed partial class IndexToBoolConverter : IValueConverter
1516
{
1617
public object Convert(object value, Type targetType, object parameter, string language)
1718
{
@@ -38,8 +39,9 @@ public object ConvertBack(object value, Type targetType, object parameter, strin
3839
}
3940
}
4041

41-
// Return -1 to indicate no change (RadioButton unchecked)
42-
return -1;
42+
// Return UnsetValue to indicate no update should occur (RadioButton unchecked)
43+
return DependencyProperty.UnsetValue;
4344
}
4445
}
4546
}
47+

‎src/settings-ui/Settings.UI/SettingsXAML/Views/PowerAccentPage.xaml‎

Lines changed: 23 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,11 @@
5151
<Setter Target="RootGrid.Background" Value="{ThemeResource ControlFillColorTertiaryBrush}"/>
5252
</VisualState.Setters>
5353
</VisualState>
54+
<VisualState x:Name="Disabled">
55+
<VisualState.Setters>
56+
<Setter Target="RootGrid.Opacity" Value="0.4"/>
57+
</VisualState.Setters>
58+
</VisualState>
5459
</VisualStateGroup>
5560
</VisualStateManager.VisualStateGroups>
5661
<Ellipse x:Name="CheckMark" Width="8" Height="8"
@@ -218,60 +223,51 @@
218223

219224
<!-- Top Left (index 5) -->
220225
<RadioButton Grid.Row="0" Grid.Column="0" GroupName="ToolbarPosition"
226+
x:Uid="QuickAccent_ToolbarPosition_TopLeftCorner"
221227
IsChecked="{x:Bind Path=ViewModel.ToolbarPositionIndex, Mode=TwoWay, Converter={StaticResource IndexToBoolConverter}, ConverterParameter=5}"
222-
Style="{StaticResource PositionRadioButtonStyle}"
223-
AutomationProperties.Name="Top left corner"
224-
ToolTipService.ToolTip="Top left corner"/>
228+
Style="{StaticResource PositionRadioButtonStyle}"/>
225229
<!-- Top Center (index 0) -->
226230
<RadioButton Grid.Row="0" Grid.Column="1" GroupName="ToolbarPosition"
231+
x:Uid="QuickAccent_ToolbarPosition_TopCenter"
227232
IsChecked="{x:Bind Path=ViewModel.ToolbarPositionIndex, Mode=TwoWay, Converter={StaticResource IndexToBoolConverter}, ConverterParameter=0}"
228-
Style="{StaticResource PositionRadioButtonStyle}"
229-
AutomationProperties.Name="Top center"
230-
ToolTipService.ToolTip="Top center"/>
233+
Style="{StaticResource PositionRadioButtonStyle}"/>
231234
<!-- Top Right (index 4) -->
232235
<RadioButton Grid.Row="0" Grid.Column="2" GroupName="ToolbarPosition"
236+
x:Uid="QuickAccent_ToolbarPosition_TopRightCorner"
233237
IsChecked="{x:Bind Path=ViewModel.ToolbarPositionIndex, Mode=TwoWay, Converter={StaticResource IndexToBoolConverter}, ConverterParameter=4}"
234-
Style="{StaticResource PositionRadioButtonStyle}"
235-
AutomationProperties.Name="Top right corner"
236-
ToolTipService.ToolTip="Top right corner"/>
238+
Style="{StaticResource PositionRadioButtonStyle}"/>
237239

238240
<!-- Left (index 2) -->
239241
<RadioButton Grid.Row="1" Grid.Column="0" GroupName="ToolbarPosition"
242+
x:Uid="QuickAccent_ToolbarPosition_Left"
240243
IsChecked="{x:Bind Path=ViewModel.ToolbarPositionIndex, Mode=TwoWay, Converter={StaticResource IndexToBoolConverter}, ConverterParameter=2}"
241-
Style="{StaticResource PositionRadioButtonStyle}"
242-
AutomationProperties.Name="Left"
243-
ToolTipService.ToolTip="Left"/>
244+
Style="{StaticResource PositionRadioButtonStyle}"/>
244245
<!-- Center (index 8) -->
245246
<RadioButton Grid.Row="1" Grid.Column="1" GroupName="ToolbarPosition"
247+
x:Uid="QuickAccent_ToolbarPosition_Center"
246248
IsChecked="{x:Bind Path=ViewModel.ToolbarPositionIndex, Mode=TwoWay, Converter={StaticResource IndexToBoolConverter}, ConverterParameter=8}"
247-
Style="{StaticResource PositionRadioButtonStyle}"
248-
AutomationProperties.Name="Center"
249-
ToolTipService.ToolTip="Center"/>
249+
Style="{StaticResource PositionRadioButtonStyle}"/>
250250
<!-- Right (index 3) -->
251251
<RadioButton Grid.Row="1" Grid.Column="2" GroupName="ToolbarPosition"
252+
x:Uid="QuickAccent_ToolbarPosition_Right"
252253
IsChecked="{x:Bind Path=ViewModel.ToolbarPositionIndex, Mode=TwoWay, Converter={StaticResource IndexToBoolConverter}, ConverterParameter=3}"
253-
Style="{StaticResource PositionRadioButtonStyle}"
254-
AutomationProperties.Name="Right"
255-
ToolTipService.ToolTip="Right"/>
254+
Style="{StaticResource PositionRadioButtonStyle}"/>
256255

257256
<!-- Bottom Left (index 7) -->
258257
<RadioButton Grid.Row="2" Grid.Column="0" GroupName="ToolbarPosition"
258+
x:Uid="QuickAccent_ToolbarPosition_BottomLeftCorner"
259259
IsChecked="{x:Bind Path=ViewModel.ToolbarPositionIndex, Mode=TwoWay, Converter={StaticResource IndexToBoolConverter}, ConverterParameter=7}"
260-
Style="{StaticResource PositionRadioButtonStyle}"
261-
AutomationProperties.Name="Bottom left corner"
262-
ToolTipService.ToolTip="Bottom left corner"/>
260+
Style="{StaticResource PositionRadioButtonStyle}"/>
263261
<!-- Bottom Center (index 1) -->
264262
<RadioButton Grid.Row="2" Grid.Column="1" GroupName="ToolbarPosition"
263+
x:Uid="QuickAccent_ToolbarPosition_BottomCenter"
265264
IsChecked="{x:Bind Path=ViewModel.ToolbarPositionIndex, Mode=TwoWay, Converter={StaticResource IndexToBoolConverter}, ConverterParameter=1}"
266-
Style="{StaticResource PositionRadioButtonStyle}"
267-
AutomationProperties.Name="Bottom center"
268-
ToolTipService.ToolTip="Bottom center"/>
265+
Style="{StaticResource PositionRadioButtonStyle}"/>
269266
<!-- Bottom Right (index 6) -->
270267
<RadioButton Grid.Row="2" Grid.Column="2" GroupName="ToolbarPosition"
268+
x:Uid="QuickAccent_ToolbarPosition_BottomRightCorner"
271269
IsChecked="{x:Bind Path=ViewModel.ToolbarPositionIndex, Mode=TwoWay, Converter={StaticResource IndexToBoolConverter}, ConverterParameter=6}"
272-
Style="{StaticResource PositionRadioButtonStyle}"
273-
AutomationProperties.Name="Bottom right corner"
274-
ToolTipService.ToolTip="Bottom right corner"/>
270+
Style="{StaticResource PositionRadioButtonStyle}"/>
275271
</Grid>
276272
</tkcontrols:SettingsCard>
277273
<tkcontrols:SettingsCard

0 commit comments

Comments
 (0)