Skip to content

Commit 0b47477

Browse files
authored
[WB-1852.2] Remove Button.light variant, modify actionStyles to support Button instances (#2563)
## Summary: This PR mainly removes the `light` prop from the `Button` component and its associated styles. The `light` prop was used to apply a light theme to the button, but it is no longer maintained and has been removed to simplify the codebase. The `inverse` action style can now be used as a replacement for these kind of scenarhos. ### Details: - Button: - Removed the `light` prop as it is no longer maintained. - Simplified pseudo-states (e.g. `:hover`, `:active`), instead of using `:hover:not("aria-disabled=true)`. - Styles: Adjusted `actionStyles.inverse` to account for `Button` instances. Issue: https://khanacademy.atlassian.net/browse/WB-1852 ## Test plan: Navigate to the `Button` component in storybook and verify that the `light` prop is no longer available. Test the `inverse` action style in storybook to ensure it looks as expected with `Button` instances. Author: jandrade Reviewers: jandrade, beaesguerra Required Reviewers: Approved By: beaesguerra Checks: ✅ 12 checks were successful, ⏭️ 2 checks have been skipped Pull Request URL: #2563
1 parent 2e50618 commit 0b47477

File tree

15 files changed

+159
-609
lines changed

15 files changed

+159
-609
lines changed

.changeset/fluffy-donuts-drop.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@khanacademy/wonder-blocks-button": major
3+
---
4+
5+
Removes the `light` prop as it is no longer maintained. Simplifes pseudo-states (e.g. `:hover`, `:active`).

.changeset/grumpy-seas-crash.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@khanacademy/wonder-blocks-styles": patch
3+
---
4+
5+
Adjust `actionStyles.inverse` to account for `Button` instances

__docs__/wonder-blocks-button/button-variants.stories.tsx

+11-63
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import {allModes} from "../../.storybook/modes";
88

99
import {PropsFor, View} from "@khanacademy/wonder-blocks-core";
1010
import {ThemeSwitcherContext} from "@khanacademy/wonder-blocks-theming";
11-
import {color, semanticColor, spacing} from "@khanacademy/wonder-blocks-tokens";
11+
import {spacing} from "@khanacademy/wonder-blocks-tokens";
1212
import {HeadingLarge, LabelMedium} from "@khanacademy/wonder-blocks-typography";
1313
import Button from "@khanacademy/wonder-blocks-button";
1414

@@ -44,29 +44,18 @@ function VariantsGroup({
4444
color = "default",
4545
disabled = false,
4646
label = "Button",
47-
light,
4847
size,
4948
}: {
5049
color?: ButtonProps["color"];
5150
disabled?: ButtonProps["disabled"];
5251
label?: string;
53-
light: boolean;
5452
size: ButtonProps["size"];
5553
}) {
56-
const theme = React.useContext(ThemeSwitcherContext);
5754
const category = disabled ? "disabled" : color;
5855

5956
return (
60-
<View
61-
style={[
62-
styles.variants,
63-
light &&
64-
(theme === "khanmigo"
65-
? styles.darkKhanmigo
66-
: styles.darkDefault),
67-
]}
68-
>
69-
<LabelMedium style={[styles.label, light && styles.inverseLabel]}>
57+
<View style={[styles.variants]}>
58+
<LabelMedium style={styles.label}>
7059
{size} / {category}
7160
</LabelMedium>
7261
{kinds.map((kind) => (
@@ -75,7 +64,6 @@ function VariantsGroup({
7564
onClick={action("clicked")}
7665
disabled={disabled}
7766
kind={kind}
78-
light={light}
7967
color={color}
8068
size={size}
8169
>
@@ -86,7 +74,6 @@ function VariantsGroup({
8674
onClick={action("clicked")}
8775
disabled={disabled}
8876
kind={kind}
89-
light={light}
9077
color={color}
9178
size={size}
9279
startIcon={paperPlaneIcon}
@@ -98,7 +85,6 @@ function VariantsGroup({
9885
onClick={action("clicked")}
9986
disabled={disabled}
10087
kind={kind}
101-
light={light}
10288
color={color}
10389
size={size}
10490
endIcon={paperPlaneIcon}
@@ -111,46 +97,17 @@ function VariantsGroup({
11197
);
11298
}
11399

114-
const KindVariants = ({light}: {light: boolean}) => {
115-
return (
116-
<>
117-
{sizes.map((size) => (
118-
<React.Fragment key={size}>
119-
{colors.map((color) => (
120-
<React.Fragment key={color}>
121-
<VariantsGroup
122-
size={size}
123-
color={color}
124-
light={light}
125-
/>
126-
{light && (
127-
<VariantsGroup
128-
size={size}
129-
color={color}
130-
disabled={true}
131-
light={light}
132-
/>
133-
)}
134-
</React.Fragment>
135-
))}
136-
{!light && (
137-
<VariantsGroup
138-
size={size}
139-
disabled={true}
140-
light={light}
141-
/>
142-
)}
143-
</React.Fragment>
144-
))}
145-
</>
146-
);
147-
};
148-
149100
const VariantsByTheme = ({themeName = "Default"}: {themeName?: string}) => (
150101
<View style={{marginBottom: spacing.large_24}}>
151102
<HeadingLarge>{themeName} theme</HeadingLarge>
152-
<KindVariants light={false} />
153-
<KindVariants light={true} />
103+
{sizes.map((size) => (
104+
<React.Fragment key={size}>
105+
{colors.map((color) => (
106+
<VariantsGroup key={color} size={size} color={color} />
107+
))}
108+
<VariantsGroup size={size} disabled={true} />
109+
</React.Fragment>
110+
))}
154111
</View>
155112
);
156113

@@ -189,12 +146,6 @@ export const Active: StoryComponentType = {
189146
};
190147

191148
const styles = StyleSheet.create({
192-
darkDefault: {
193-
backgroundColor: color.darkBlue,
194-
},
195-
darkKhanmigo: {
196-
backgroundColor: color.eggplant,
197-
},
198149
variants: {
199150
justifyContent: "flex-start",
200151
padding: spacing.medium_16,
@@ -206,7 +157,4 @@ const styles = StyleSheet.create({
206157
label: {
207158
minWidth: 150,
208159
},
209-
inverseLabel: {
210-
color: semanticColor.text.inverse,
211-
},
212160
});

__docs__/wonder-blocks-button/button.argtypes.ts

-116
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,9 @@ import {IconMappings} from "../wonder-blocks-icon/phosphor-icon.argtypes";
44

55
export default {
66
children: {
7-
description: "Text to appear on the button.",
87
type: {name: "string", required: true},
98
},
109
startIcon: {
11-
description: `A Phosphor icon asset (imported as a static SVG file)
12-
that will appear at the start of the button (left for LTR, right
13-
for RTL).`,
14-
type: {name: "other", value: "PhosphorIconAsset", required: false},
15-
control: {type: "select"},
1610
options: Object.keys(IconMappings),
1711
mapping: IconMappings,
1812
table: {
@@ -21,11 +15,6 @@ export default {
2115
},
2216
},
2317
endIcon: {
24-
description: `A Phosphor icon asset (imported as a static SVG file)
25-
that will appear at the end of the button (right for LTR, left
26-
for RTL).`,
27-
type: {name: "other", value: "PhosphorIconAsset", required: false},
28-
control: {type: "select"},
2918
options: Object.keys(IconMappings),
3019
mapping: IconMappings,
3120
table: {
@@ -34,104 +23,21 @@ export default {
3423
},
3524
},
3625
spinner: {
37-
description: "If true, replaces the contents with a spinner.",
3826
control: {type: "boolean"},
3927
table: {
4028
category: "Layout",
41-
type: {
42-
summary: "boolean",
43-
detail: "Setting this prop to `true` will disable the button.",
44-
},
45-
},
46-
},
47-
color: {
48-
description: "The color of the button, either blue or red.",
49-
options: ["default", "destructive"],
50-
control: {type: "radio"},
51-
table: {
52-
category: "Theming",
53-
type: {
54-
summary: `"default" | "destructive"`,
55-
},
56-
},
57-
},
58-
kind: {
59-
description:
60-
"The kind of the button, either primary, secondary, or tertiary.",
61-
options: ["primary", "secondary", "tertiary"],
62-
control: {type: "select"},
63-
table: {
64-
type: {summary: "primary | secondary | tertiary"},
65-
defaultValue: {
66-
detail: `
67-
- Primary buttons have background colors.\n- Secondary buttons have a border and no background color.\n- Tertiary buttons have no background or border.
68-
`,
69-
},
70-
},
71-
},
72-
light: {
73-
description: "Whether the button is on a dark/colored background.",
74-
control: {type: "boolean"},
75-
table: {
76-
category: "Theming",
77-
type: {
78-
summary: "boolean",
79-
detail: "Sets primary button background color to white, and secondary and tertiary button title to color.",
80-
},
8129
},
8230
},
8331
size: {
84-
description: "The size of the button.",
85-
options: ["small", "medium", "large"],
8632
control: {type: "select"},
8733
table: {
8834
category: "Layout",
89-
defaultValue: {
90-
detail: `"medium" = height: 40; "small" = height: 32; "large" = height: 56;`,
91-
},
9235
type: {
9336
summary: `"medium" | "small" | "large"`,
9437
},
9538
},
9639
},
97-
disabled: {
98-
description: "Whether the button is disabled.",
99-
table: {
100-
type: {
101-
summary: "boolean",
102-
},
103-
},
104-
},
105-
id: {
106-
description: "An optional id attribute.",
107-
control: {type: "text"},
108-
table: {
109-
type: {
110-
summary: "string",
111-
},
112-
},
113-
},
114-
testId: {
115-
description: "Test ID used for e2e testing.",
116-
control: {type: "text"},
117-
table: {
118-
type: {
119-
summary: "string",
120-
},
121-
},
122-
},
123-
124-
tabIndex: {
125-
description: "Set the tabindex attribute on the rendered element.",
126-
control: {type: "number", min: -1},
127-
table: {
128-
type: {
129-
summary: "number",
130-
},
131-
},
132-
},
13340
style: {
134-
description: "Optional custom styles.",
13541
table: {
13642
category: "Layout",
13743
type: {
@@ -140,7 +46,6 @@ export default {
14046
},
14147
},
14248
labelStyle: {
143-
description: "Optional custom styles for the inner label.",
14449
table: {
14550
category: "Layout",
14651
type: {
@@ -149,7 +54,6 @@ export default {
14954
},
15055
},
15156
className: {
152-
description: "Adds CSS classes to the Button.",
15357
control: {type: "text"},
15458
table: {
15559
category: "Layout",
@@ -163,9 +67,6 @@ export default {
16367
*/
16468
onClick: {
16569
action: "clicked",
166-
description: `Function to call when button is clicked.
167-
This callback should be used for things like marking BigBingo conversions. It should NOT be used to redirect to a different URL or to prevent navigation via e.preventDefault(). The event passed to this handler will have its preventDefault() and stopPropagation() methods stubbed out.
168-
`,
16970
table: {
17071
category: "Events",
17172
type: {
@@ -179,39 +80,24 @@ export default {
17980
* Navigation
18081
*/
18182
skipClientNav: {
182-
description: `Whether to avoid using client-side navigation. If the URL passed to href is local to the client-side, e.g. /math/algebra/eval-exprs, then it tries to use react-router-dom's Link component which handles the client-side navigation. You can set "skipClientNav" to true avoid using client-side nav entirely.`,
18383
control: {type: "boolean"},
18484
table: {
18585
category: "Navigation",
186-
type: {
187-
summary: "Note",
188-
detail: "All URLs containing a protocol are considered external, e.g. https://khanacademy.org/math/algebra/eval-exprs will trigger a full page reload.",
189-
},
19086
},
19187
},
19288
rel: {
193-
description: `Specifies the type of relationship between the current document and the linked document. Should only be used when "href" is specified. This defaults to "noopener noreferrer" when target="_blank", but can be overridden by setting this prop to something else.`,
19489
control: {type: "text"},
19590
table: {
19691
category: "Navigation",
197-
type: {
198-
summary: "string",
199-
},
20092
},
20193
},
20294
target: {
203-
description: `A target destination window for a link to open in. Should only be used
204-
* when "href" is specified.`,
20595
control: {type: "text"},
20696
table: {
20797
category: "Navigation",
208-
type: {
209-
summary: "string",
210-
},
21198
},
21299
},
213100
href: {
214-
description: "URL to navigate to.",
215101
control: {type: "text"},
216102
table: {
217103
category: "Navigation",
@@ -222,7 +108,6 @@ export default {
222108
},
223109
},
224110
beforeNav: {
225-
description: `Run async code before navigating. If the promise returned rejects then navigation will not occur. If both safeWithNav and beforeNav are provided, beforeNav will be run first and safeWithNav will only be run if beforeNav does not reject.`,
226111
table: {
227112
category: "Navigation",
228113
type: {
@@ -231,7 +116,6 @@ export default {
231116
},
232117
},
233118
safeWithNav: {
234-
description: `Run async code in the background while client-side navigating. If the browser does a full page load navigation, the callback promise must be settled before the navigation will occur. Errors are ignored so that navigation is guaranteed to succeed.`,
235119
table: {
236120
category: "Navigation",
237121
type: {

0 commit comments

Comments
 (0)