Skip to content

Commit 168c787

Browse files
chrisbobbechrisbobbe
committed
button [nfc]: Have ZulipWebUiKitButton support a smaller, ad hoc size
For muted-users, coming up. This was ad hoc for mobile, for the "Reveal message" button on a message from a muted sender: https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=6092-50786&m=dev
1 parent cdcb1bc commit 168c787

File tree

2 files changed

+120
-77
lines changed

2 files changed

+120
-77
lines changed

‎lib/widgets/button.dart

Lines changed: 41 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,14 @@ class ZulipWebUiKitButton extends StatelessWidget {
1818
super.key,
1919
this.attention = ZulipWebUiKitButtonAttention.medium,
2020
this.intent = ZulipWebUiKitButtonIntent.info,
21+
this.size = ZulipWebUiKitButtonSize.normal,
2122
required this.label,
2223
required this.onPressed,
2324
});
2425

2526
final ZulipWebUiKitButtonAttention attention;
2627
final ZulipWebUiKitButtonIntent intent;
28+
final ZulipWebUiKitButtonSize size;
2729
final String label;
2830
final VoidCallback onPressed;
2931

@@ -53,19 +55,23 @@ class ZulipWebUiKitButton extends StatelessWidget {
5355

5456
TextStyle _labelStyle(BuildContext context, {required TextScaler textScaler}) {
5557
final designVariables = DesignVariables.of(context);
56-
// Values chosen from the Figma frame for zulip-flutter's compose box:
58+
// Normal-size values chosen from the Figma frame for zulip-flutter's
59+
// compose box:
5760
// https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=3988-38201&m=dev
5861
// Commented values come from the Figma page "Zulip Web UI kit":
5962
// https://www.figma.com/design/msWyAJ8cnMHgOMPxi7BUvA/Zulip-Web-UI-kit?node-id=1-8&p=f&m=dev
6063
// Discussion:
6164
// https://github.com/zulip/zulip-flutter/pull/1432#discussion_r2023880851
6265
return TextStyle(
6366
color: _labelColor(designVariables),
64-
fontSize: 17, // 16
65-
height: 1.20, // 1.25
66-
letterSpacing: proportionalLetterSpacing(context, textScaler: textScaler,
67-
0.006,
68-
baseFontSize: 17), // 16
67+
fontSize: _forSize(16, 17 /* 16 */),
68+
height: _forSize(1, 1.20 /* 1.25 */),
69+
letterSpacing: _forSize(
70+
0,
71+
proportionalLetterSpacing(context, textScaler: textScaler,
72+
0.006,
73+
baseFontSize: 17 /* 16 */),
74+
),
6975
).merge(weightVariableTextStyle(context,
7076
wght: 600)); // 500
7177
}
@@ -87,6 +93,12 @@ class ZulipWebUiKitButton extends StatelessWidget {
8793
}
8894
}
8995

96+
T _forSize<T>(T small, T normal) =>
97+
switch (size) {
98+
ZulipWebUiKitButtonSize.small => small,
99+
ZulipWebUiKitButtonSize.normal => normal,
100+
};
101+
90102
@override
91103
Widget build(BuildContext context) {
92104
final designVariables = DesignVariables.of(context);
@@ -104,24 +116,32 @@ class ZulipWebUiKitButton extends StatelessWidget {
104116
// from shrinking to zero as the button grows to accommodate a larger label
105117
final textScaler = MediaQuery.textScalerOf(context).clamp(maxScaleFactor: 1.5);
106118

119+
final buttonHeight = _forSize(24, 28);
120+
107121
return AnimatedScaleOnTap(
108122
scaleEnd: 0.96,
109123
duration: Duration(milliseconds: 100),
110124
child: TextButton(
111125
style: TextButton.styleFrom(
112-
padding: EdgeInsets.symmetric(horizontal: 10, vertical: 4 - densityVerticalAdjustment),
126+
padding: EdgeInsets.symmetric(
127+
horizontal: _forSize(6, 10),
128+
vertical: 4 - densityVerticalAdjustment,
129+
),
113130
foregroundColor: _labelColor(designVariables),
114131
shape: RoundedRectangleBorder(
115132
side: _borderSide(designVariables),
116-
borderRadius: BorderRadius.circular(4)),
133+
borderRadius: BorderRadius.circular(_forSize(6, 4))),
117134
splashFactory: NoSplash.splashFactory,
118135

119-
// These three arguments make the button 28px tall vertically,
136+
// These three arguments make the button `buttonHeight` tall,
120137
// but with vertical padding to make the touch target 44px tall:
121138
// https://github.com/zulip/zulip-flutter/pull/1432#discussion_r2023907300
122139
visualDensity: visualDensity,
123140
tapTargetSize: MaterialTapTargetSize.padded,
124-
minimumSize: Size(kMinInteractiveDimension, 28 - densityVerticalAdjustment),
141+
minimumSize: Size(
142+
kMinInteractiveDimension,
143+
buttonHeight - densityVerticalAdjustment,
144+
),
125145
).copyWith(backgroundColor: _backgroundColor(designVariables)),
126146
onPressed: onPressed,
127147
child: ConstrainedBox(
@@ -150,6 +170,17 @@ enum ZulipWebUiKitButtonIntent {
150170
// brand,
151171
}
152172

173+
enum ZulipWebUiKitButtonSize {
174+
/// A smaller size than the one in the Zulip Web UI Kit.
175+
///
176+
/// This was ad hoc for mobile, for the "Reveal message" button
177+
/// on a message from a muted sender:
178+
/// https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=6092-50786&m=dev
179+
small,
180+
181+
normal,
182+
}
183+
153184
/// Apply [Transform.scale] to the child widget when tapped, and reset its scale
154185
/// when released, while animating the transitions.
155186
class AnimatedScaleOnTap extends StatefulWidget {

‎test/widgets/button_test.dart

Lines changed: 79 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -16,72 +16,84 @@ void main() {
1616
TestZulipBinding.ensureInitialized();
1717

1818
group('ZulipWebUiKitButton', () {
19-
final textScaleFactorVariants = ValueVariant(Set.of(kTextScaleFactors));
20-
21-
testWidgets('vertical outer padding is preserved as text scales', (tester) async {
22-
addTearDown(testBinding.reset);
23-
tester.platformDispatcher.textScaleFactorTestValue = textScaleFactorVariants.currentValue!;
24-
addTearDown(tester.platformDispatcher.clearTextScaleFactorTestValue);
25-
26-
final buttonFinder = find.byType(ZulipWebUiKitButton);
27-
28-
await tester.pumpWidget(TestZulipApp(
29-
child: UnconstrainedBox(
30-
child: ZulipWebUiKitButton(label: 'Cancel', onPressed: () {}))));
31-
await tester.pump();
32-
33-
final element = tester.element(buttonFinder);
34-
final renderObject = element.renderObject as RenderBox;
35-
final size = renderObject.size;
36-
check(size).height.equals(44); // includes outer padding
37-
38-
final textScaler = TextScaler.linear(textScaleFactorVariants.currentValue!)
39-
.clamp(maxScaleFactor: 1.5);
40-
final expectedButtonHeight = max(28.0, // configured min height
41-
(textScaler.scale(17) * 1.20).roundToDouble() // text height
42-
+ 4 + 4); // vertical padding
43-
44-
// Rounded rectangle paints with the intended height…
45-
final expectedRRect = RRect.fromLTRBR(
46-
0, 0, // zero relative to the position at this paint step
47-
size.width, expectedButtonHeight, Radius.circular(4));
48-
check(renderObject).legacyMatcher(
49-
// `paints` isn't a [Matcher] so we wrap it with `equals`;
50-
// awkward but it works
51-
equals(paints..drrect(outer: expectedRRect)));
52-
53-
// …and that height leaves at least 4px for vertical outer padding.
54-
check(expectedButtonHeight).isLessOrEqual(44 - 2 - 2);
55-
}, variant: textScaleFactorVariants);
56-
57-
testWidgets('vertical outer padding responds to taps, not just painted area', (tester) async {
58-
addTearDown(testBinding.reset);
59-
tester.platformDispatcher.textScaleFactorTestValue = textScaleFactorVariants.currentValue!;
60-
addTearDown(tester.platformDispatcher.clearTextScaleFactorTestValue);
61-
62-
final buttonFinder = find.byType(ZulipWebUiKitButton);
63-
64-
int numTapsHandled = 0;
65-
await tester.pumpWidget(TestZulipApp(
66-
child: UnconstrainedBox(
67-
child: ZulipWebUiKitButton(
68-
label: 'Cancel',
69-
onPressed: () => numTapsHandled++))));
70-
await tester.pump();
71-
72-
final element = tester.element(buttonFinder);
73-
final renderObject = element.renderObject as RenderBox;
74-
final size = renderObject.size;
75-
check(size).height.equals(44); // includes outer padding
76-
77-
// Outer padding responds to taps, not just the painted part.
78-
final buttonCenter = tester.getCenter(buttonFinder);
79-
int numTaps = 0;
80-
for (double y = -22; y < 22; y++) {
81-
await tester.tapAt(buttonCenter + Offset(0, y));
82-
numTaps++;
83-
}
84-
check(numTapsHandled).equals(numTaps);
85-
}, variant: textScaleFactorVariants);
19+
void testVerticalOuterPadding({required ZulipWebUiKitButtonSize sizeVariant}) {
20+
final textScaleFactorVariants = ValueVariant(Set.of(kTextScaleFactors));
21+
T forSizeVariant<T>(T small, T normal) =>
22+
switch (sizeVariant) {
23+
ZulipWebUiKitButtonSize.small => small,
24+
ZulipWebUiKitButtonSize.normal => normal,
25+
};
26+
27+
testWidgets('vertical outer padding is preserved as text scales; $sizeVariant', (tester) async {
28+
addTearDown(testBinding.reset);
29+
tester.platformDispatcher.textScaleFactorTestValue = textScaleFactorVariants.currentValue!;
30+
addTearDown(tester.platformDispatcher.clearTextScaleFactorTestValue);
31+
32+
final buttonFinder = find.byType(ZulipWebUiKitButton);
33+
34+
await tester.pumpWidget(TestZulipApp(
35+
child: UnconstrainedBox(
36+
child: ZulipWebUiKitButton(
37+
label: 'Cancel',
38+
onPressed: () {},
39+
size: sizeVariant))));
40+
await tester.pump();
41+
42+
final element = tester.element(buttonFinder);
43+
final renderObject = element.renderObject as RenderBox;
44+
final size = renderObject.size;
45+
check(size).height.equals(44); // includes outer padding
46+
47+
final textScaler = TextScaler.linear(textScaleFactorVariants.currentValue!)
48+
.clamp(maxScaleFactor: 1.5);
49+
final expectedButtonHeight = max(forSizeVariant(24.0, 28.0), // configured min height
50+
(textScaler.scale(forSizeVariant(16, 17) * forSizeVariant(1, 1.20)).roundToDouble() // text height
51+
+ 4 + 4)); // vertical padding
52+
53+
// Rounded rectangle paints with the intended height…
54+
final expectedRRect = RRect.fromLTRBR(
55+
0, 0, // zero relative to the position at this paint step
56+
size.width, expectedButtonHeight, Radius.circular(forSizeVariant(6, 4)));
57+
check(renderObject).legacyMatcher(
58+
// `paints` isn't a [Matcher] so we wrap it with `equals`;
59+
// awkward but it works
60+
equals(paints..drrect(outer: expectedRRect)));
61+
62+
// …and that height leaves at least 4px for vertical outer padding.
63+
check(expectedButtonHeight).isLessOrEqual(44 - 2 - 2);
64+
}, variant: textScaleFactorVariants);
65+
66+
testWidgets('vertical outer padding responds to taps, not just painted area', (tester) async {
67+
addTearDown(testBinding.reset);
68+
tester.platformDispatcher.textScaleFactorTestValue = textScaleFactorVariants.currentValue!;
69+
addTearDown(tester.platformDispatcher.clearTextScaleFactorTestValue);
70+
71+
final buttonFinder = find.byType(ZulipWebUiKitButton);
72+
73+
int numTapsHandled = 0;
74+
await tester.pumpWidget(TestZulipApp(
75+
child: UnconstrainedBox(
76+
child: ZulipWebUiKitButton(
77+
label: 'Cancel',
78+
onPressed: () => numTapsHandled++))));
79+
await tester.pump();
80+
81+
final element = tester.element(buttonFinder);
82+
final renderObject = element.renderObject as RenderBox;
83+
final size = renderObject.size;
84+
check(size).height.equals(44); // includes outer padding
85+
86+
// Outer padding responds to taps, not just the painted part.
87+
final buttonCenter = tester.getCenter(buttonFinder);
88+
int numTaps = 0;
89+
for (double y = -22; y < 22; y++) {
90+
await tester.tapAt(buttonCenter + Offset(0, y));
91+
numTaps++;
92+
}
93+
check(numTapsHandled).equals(numTaps);
94+
}, variant: textScaleFactorVariants);
95+
}
96+
testVerticalOuterPadding(sizeVariant: ZulipWebUiKitButtonSize.small);
97+
testVerticalOuterPadding(sizeVariant: ZulipWebUiKitButtonSize.normal);
8698
});
8799
}

0 commit comments

Comments
 (0)