Skip to content

Commit b724356

Browse files
rurikoarakirurikoaraki
authored
[ButtonV1][Win32] Fix focus borders on primary buttons (#2693)
* Some attempts * More tests * More tests * Trying more things * More trying things * Logic cleanup * Fix bug * Fix bug * Revert tests * Add more test cases * Fix bug * Fix bugs * Make changes to compound button * Make button fixes * Copy of CompoundButtonTokens for win32 * Undo changes to main compound button file * Compound button fixes * Change files * Revert some changes * Revert some changes * Update snapshots * Code cleanup * Code cleanup * Change files * Fix HC bug * Rename state variables * Perf improvement, rename variable * Address feedback
1 parent 3512da9 commit b724356

26 files changed

+448
-19
lines changed

‎apps/fluent-tester/src/TestComponents/Button/ButtonShapeTestSection.tsx

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,15 @@ export const ButtonShapeTest: React.FunctionComponent = () => {
3535
<CompoundButton secondaryContent="circular" shape="circular" style={commonTestStyles.vmargin}>
3636
Compound Button
3737
</CompoundButton>
38+
<CompoundButton appearance="primary" secondaryContent="rounded" shape="rounded" style={commonTestStyles.vmargin}>
39+
Compound Button
40+
</CompoundButton>
41+
<CompoundButton appearance="primary" secondaryContent="square" shape="square" style={commonTestStyles.vmargin}>
42+
Compound Button
43+
</CompoundButton>
44+
<CompoundButton appearance="primary" secondaryContent="circular" shape="circular" style={commonTestStyles.vmargin}>
45+
Compound Button
46+
</CompoundButton>
3847
</View>
3948
);
4049
};

‎apps/fluent-tester/src/TestComponents/Button/ButtonSizeTestSection.tsx

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,29 +23,63 @@ export const ButtonSizeTest: React.FunctionComponent = () => {
2323
style={commonTestStyles.vmargin}
2424
tooltip="button tooltip"
2525
/>
26+
<Button
27+
iconOnly
28+
size="small"
29+
appearance="primary"
30+
icon={iconProps}
31+
accessibilityLabel="Small size button with accessibility icon"
32+
style={commonTestStyles.vmargin}
33+
tooltip="button tooltip"
34+
/>
2635
<Button
2736
iconOnly
2837
size="medium"
2938
icon={iconProps}
3039
accessibilityLabel="Medium size button with accessibility icon"
3140
style={commonTestStyles.vmargin}
3241
/>
42+
<Button
43+
iconOnly
44+
size="medium"
45+
appearance="primary"
46+
icon={iconProps}
47+
accessibilityLabel="Medium size button with accessibility icon"
48+
style={commonTestStyles.vmargin}
49+
/>
50+
<Button
51+
iconOnly
52+
size="large"
53+
icon={iconProps}
54+
accessibilityLabel="Large size button with accessibility icon"
55+
style={commonTestStyles.vmargin}
56+
/>
3357
<Button
3458
iconOnly
3559
size="large"
60+
appearance="primary"
3661
icon={iconProps}
3762
accessibilityLabel="Large size button with accessibility icon"
3863
style={commonTestStyles.vmargin}
3964
/>
4065
<Button size="small" icon={iconProps} style={commonTestStyles.vmargin}>
4166
Small Button with icon
4267
</Button>
68+
<Button size="small" appearance="primary" icon={iconProps} style={commonTestStyles.vmargin}>
69+
Small Button with icon
70+
</Button>
4371
<Button size="medium" icon={iconProps} style={commonTestStyles.vmargin}>
4472
Medium Button with icon
4573
</Button>
74+
<Button size="medium" appearance="primary" icon={iconProps} style={commonTestStyles.vmargin}>
75+
Medium Button with icon
76+
</Button>
4677
<Button size="large" icon={iconProps} style={commonTestStyles.vmargin}>
4778
Large Button with icon
4879
</Button>
80+
<Button size="large" appearance="primary" icon={iconProps} style={commonTestStyles.vmargin}>
81+
Large Button with icon
82+
</Button>
4983
{Platform.OS == 'android' && (
5084
<FAB size="small" icon={iconProps} style={commonTestStyles.vmargin}>
5185
Small FAB
@@ -79,12 +113,21 @@ export const ButtonSizeTest: React.FunctionComponent = () => {
79113
<CompoundButton secondaryContent="Small compound button" size="small" style={commonTestStyles.vmargin}>
80114
Compound Button
81115
</CompoundButton>
116+
<CompoundButton appearance="primary" secondaryContent="Small compound button" size="small" style={commonTestStyles.vmargin}>
117+
Compound Button
118+
</CompoundButton>
82119
<CompoundButton secondaryContent="Medium compound button" size="medium" style={commonTestStyles.vmargin}>
83120
Compound Button
84121
</CompoundButton>
122+
<CompoundButton appearance="primary" secondaryContent="Medium compound button" size="medium" style={commonTestStyles.vmargin}>
123+
Compound Button
124+
</CompoundButton>
85125
<CompoundButton secondaryContent="Large compound button" size="large" style={commonTestStyles.vmargin}>
86126
Compound Button
87127
</CompoundButton>
128+
<CompoundButton appearance="primary" secondaryContent="Large compound button" size="large" style={commonTestStyles.vmargin}>
129+
Compound Button
130+
</CompoundButton>
88131
{svgIconsEnabled && (
89132
<>
90133
<CompoundButton icon={iconProps} secondaryContent="SecondaryContent" size="small" style={commonTestStyles.vmargin}>

‎change/@fluentui-react-native-button-0fd410c6-ed7e-4d3f-9c6b-045b6685b7e0.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "patch",
3+
"comment": "Fix focus border for primary button on win32",
4+
"packageName": "@fluentui-react-native/button",
5+
"email": "[email protected]",
6+
"dependentChangeType": "patch"
7+
}

‎change/@fluentui-react-native-experimental-menu-button-6b0c1cac-2a0e-4a72-9702-f3c289a4e541.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "patch",
3+
"comment": "Update snapshots",
4+
"packageName": "@fluentui-react-native/experimental-menu-button",
5+
"email": "[email protected]",
6+
"dependentChangeType": "patch"
7+
}

‎change/@fluentui-react-native-menu-button-046099b8-7a50-40a6-8b62-ea4783fa21ae.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "patch",
3+
"comment": "Code cleanup",
4+
"packageName": "@fluentui-react-native/menu-button",
5+
"email": "[email protected]",
6+
"dependentChangeType": "patch"
7+
}

‎change/@fluentui-react-native-menu-d56e7525-27b7-4dd9-9017-8251bb3abfbc.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "patch",
3+
"comment": "Update snapshots",
4+
"packageName": "@fluentui-react-native/menu",
5+
"email": "[email protected]",
6+
"dependentChangeType": "patch"
7+
}

‎change/@fluentui-react-native-notification-4f5d22c4-bc32-4e07-b213-03dd2f83ef79.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "patch",
3+
"comment": "Update snapshots",
4+
"packageName": "@fluentui-react-native/notification",
5+
"email": "[email protected]",
6+
"dependentChangeType": "patch"
7+
}

‎change/@fluentui-react-native-tester-06c923f7-ce34-4a03-b480-daad63e4e299.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "patch",
3+
"comment": "Add more test cases",
4+
"packageName": "@fluentui-react-native/tester",
5+
"email": "[email protected]",
6+
"dependentChangeType": "patch"
7+
}

‎packages/components/Button/src/Button.styling.ts

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,19 +15,19 @@ import { defaultButtonTokens } from './ButtonTokens';
1515

1616
export const buttonStates: (keyof ButtonTokens)[] = [
1717
'block',
18-
'primary',
19-
'subtle',
20-
'outline',
21-
'hovered',
2218
'small',
2319
'medium',
2420
'large',
2521
'hasContent',
2622
'hasIconAfter',
2723
'hasIconBefore',
24+
'primary',
25+
'subtle',
26+
'outline',
2827
'rounded',
2928
'circular',
3029
'square',
30+
'hovered',
3131
'focused',
3232
'pressed',
3333
'disabled',
@@ -91,6 +91,17 @@ export const stylingSettings: UseStylingOptions<ButtonProps, ButtonSlotProps, Bu
9191
}),
9292
['iconColor', 'iconSize'],
9393
),
94+
focusInnerBorder: buildProps(
95+
(tokens: ButtonTokens) => ({
96+
style: {
97+
position: 'absolute',
98+
borderWidth: tokens.borderInnerWidth,
99+
borderColor: tokens.borderInnerColor,
100+
borderRadius: tokens.borderInnerRadius,
101+
},
102+
}),
103+
['borderInnerWidth', 'borderInnerColor', 'borderInnerRadius'],
104+
),
94105
},
95106
};
96107

‎packages/components/Button/src/Button.tsx

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { Platform, Pressable, View } from 'react-native';
44

55
import { ActivityIndicator } from '@fluentui-react-native/experimental-activity-indicator';
66
import type { UseSlots } from '@fluentui-react-native/framework';
7-
import { compose, mergeProps, withSlots } from '@fluentui-react-native/framework';
7+
import { compose, memoize, mergeProps, withSlots } from '@fluentui-react-native/framework';
88
import { Icon, createIconProps } from '@fluentui-react-native/icon';
99
import type { IPressableState } from '@fluentui-react-native/interactive-hooks';
1010
import { TextV1 as Text } from '@fluentui-react-native/text';
@@ -44,7 +44,8 @@ export const Button = compose<ButtonType>({
4444
...stylingSettings,
4545
slots: {
4646
root: Pressable,
47-
rippleContainer: View,
47+
rippleContainer: Platform.OS === 'android' && View,
48+
focusInnerBorder: Platform.OS === ('win32' as any) && View,
4849
icon: Icon,
4950
content: Text,
5051
},
@@ -110,9 +111,26 @@ export const Button = compose<ButtonType>({
110111
return (
111112
<Slots.root {...mergedProps} accessibilityLabel={label}>
112113
{buttonContent}
114+
{button.state.focused && button.state.shouldUseTwoToneFocusBorder && (
115+
<Slots.focusInnerBorder
116+
style={getFocusBorderStyle(button.state.measuredHeight, button.state.measuredWidth)}
117+
accessible={false}
118+
focusable={false}
119+
/>
120+
)}
113121
</Slots.root>
114122
);
115123
}
116124
};
117125
},
118126
});
127+
128+
const getFocusBorderStyleWorker = (height: number, width: number) => {
129+
const adjustment = 2; // width of border * 2
130+
131+
return {
132+
height: height - adjustment,
133+
width: width - adjustment,
134+
};
135+
};
136+
export const getFocusBorderStyle = memoize(getFocusBorderStyleWorker);

‎packages/components/Button/src/Button.types.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ export interface ButtonCoreTokens extends LayoutTokens, FontTokens, IBorderToken
5959
shadowToken?: ShadowToken;
6060

6161
/**
62-
* Focused State on Android has inner and outer borders.
62+
* Focused State on Android and win32 primary has inner and outer borders.
6363
* Outer Border is equivalent to the border tokens from IBorders.
6464
*/
6565
borderInnerColor?: ColorValue;
@@ -166,14 +166,23 @@ export interface ButtonProps extends ButtonCoreProps {
166166
loading?: boolean;
167167
}
168168

169+
interface ButtonState extends PressableState {
170+
measuredHeight?: number;
171+
measuredWidth?: number;
172+
173+
// win32 only. Whether the component should use a tone-tone focus border instead of single-tone
174+
shouldUseTwoToneFocusBorder?: boolean;
175+
}
176+
169177
export interface ButtonInfo {
170178
props: ButtonProps & React.ComponentPropsWithRef<any>;
171-
state: PressableState;
179+
state: ButtonState;
172180
}
173181

174182
export interface ButtonSlotProps {
175183
root: React.PropsWithRef<PressablePropsExtended>;
176184
rippleContainer?: IViewProps; // Android only
185+
focusInnerBorder?: IViewProps; // Win32 only
177186
icon: IconProps;
178187
content: TextProps;
179188
}

‎packages/components/Button/src/ButtonColorTokens.win32.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,8 @@ export const defaultButtonColorTokens: TokenSettings<ButtonTokens, Theme> = (t:
6666
focused: {
6767
backgroundColor: t.colors.brandBackgroundHover,
6868
color: t.colors.neutralForegroundOnBrandHover,
69-
borderColor: t.colors.transparentStroke,
69+
borderColor: t.colors.strokeFocus2,
70+
borderInnerColor: t.colors.strokeFocus1,
7071
iconColor: t.colors.neutralForegroundOnBrandHover,
7172
},
7273
},

0 commit comments

Comments
 (0)