Skip to content

Commit

Permalink
WB-1645.pre: Append className value in Text component (fix) (#2138)
Browse files Browse the repository at this point in the history
## Summary:

While working on adding custom option items to Dropdowns, I noticed that the
`className` prop was not being appended to the `Text` component. This PR fixes
that by changing the logic to append the `className` prop to the `Text` if it
includes both a `className` and a `style` prop.

Also updated the typography docs to include an example of using the `style` and
`className` props.

Issue: WB-1645

## Test plan:

Verify that the unit tests pass.

Also verify that the following story works as expected:

http://localhost:6061/?path=/docs/typography--docs#with-style

1. Verify that the text is blue (assigned via `style`).
2. Verify that the has a bg color (assigned via `className` + `css()`).
3. Lastly, verify that it includes a black border (assigned via `className` + and external css import).

<img width="864" alt="Screenshot 2023-12-13 at 11 29 24 AM" src="https://github.com/Khan/wonder-blocks/assets/843075/aaf73683-ab6d-41ee-b1f2-7bd6d85abc49">

Author: jandrade

Reviewers: jeresig

Required Reviewers:

Approved By: jeresig

Checks: ✅ Chromatic - Get results on regular PRs (ubuntu-latest, 16.x), ✅ codecov/project, ✅ Test (ubuntu-latest, 16.x, 2/2), ✅ Check build sizes (ubuntu-latest, 16.x), ✅ Test (ubuntu-latest, 16.x, 1/2), ✅ Lint (ubuntu-latest, 16.x), ✅ Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 16.x), ⏭  Chromatic - Skip on Release PR (changesets), 🚫 Chromatic - Get results on regular PRs, ✅ Test (ubuntu-latest, 16.x, 2/2), ✅ Test (ubuntu-latest, 16.x, 1/2), ✅ Check build sizes (ubuntu-latest, 16.x), ✅ Lint (ubuntu-latest, 16.x), ✅ Publish npm snapshot (ubuntu-latest, 16.x), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 16.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 16.x), ✅ gerald, 🚫 Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 16.x), ⏭  Chromatic - Skip on Release PR (changesets), 🚫 Chromatic - Get results on regular PRs, ✅ Publish npm snapshot (ubuntu-latest, 16.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 16.x), ✅ gerald, ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 16.x), ✅ Test (ubuntu-latest, 16.x, 2/2), ✅ Check build sizes (ubuntu-latest, 16.x), ✅ Test (ubuntu-latest, 16.x, 1/2), ✅ Lint (ubuntu-latest, 16.x), 🚫 Chromatic - Build on regular PRs / chromatic (ubuntu-latest, 16.x), ⏭  Chromatic - Skip on Release PR (changesets)

Pull Request URL: #2138
  • Loading branch information
jandrade authored Dec 14, 2023
1 parent 70e846c commit 6df21f7
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 19 deletions.
5 changes: 5 additions & 0 deletions .changeset/stale-steaks-call.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/wonder-blocks-core": patch
---

Append className if set via props
6 changes: 6 additions & 0 deletions __docs__/wonder-blocks-typography/styles.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
.custom-style {
border: 1px solid #000;
/* NOTE: The color style is not applied to the text because the styles
defined via Aphrodite take more precedence */
color: red!important;
}
49 changes: 34 additions & 15 deletions __docs__/wonder-blocks-typography/typography.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as React from "react";
import {StyleSheet} from "aphrodite";
import {css, StyleSheet} from "aphrodite";
import type {Meta, StoryObj} from "@storybook/react";

import Color from "@khanacademy/wonder-blocks-color";
Expand Down Expand Up @@ -29,6 +29,10 @@ import packageConfig from "../../packages/wonder-blocks-typography/package.json"
import ComponentInfo from "../../.storybook/components/component-info";
import TypographyArgTypes from "./typography.argtypes";

// NOTE: Only for testing purposes.
// eslint-disable-next-line import/no-unassigned-import
import "./styles.css";

const typographyDescription = `Typography. \`wonder-blocks-typography\`
provides a set of standardized components for displaying text in a consistent
way. This includes components for headings, paragraphs, and text
Expand Down Expand Up @@ -101,21 +105,36 @@ TypographyElements.parameters = {
},
};

export const WithStyle: StoryObj<typeof Title> = () => {
const styles = StyleSheet.create({
blueText: {
color: Color.blue,
},
});

return <Title style={styles.blueText}>Blue Title</Title>;
};
/**
* You can change the color of text using the following patterns:
*
* 1. Via the `style` prop. This is our recommended approach.
* 2. Via the `className` prop. This is not recommended, but it is supported.
* - You can use the `css` function from `aphrodite` to create a class name
* that you can pass to the `className` prop.
* - You can pass a string to the `className` prop. This is not recommended
* and should only be used as a last resort if the other options don't cover
* your use case.
*/
export const WithStyle: StoryObj<typeof Title> = {
render: () => {
const styles = StyleSheet.create({
blueText: {
color: Color.blue,
},
highlighted: {
background: Color.offBlack16,
},
});

WithStyle.parameters = {
docs: {
description: {
story: "You can change the color of text with the `style` prop.",
},
return (
<Title
className={`${css(styles.highlighted)} custom-style`}
style={styles.blueText}
>
Blue Title
</Title>
);
},
};

Expand Down
33 changes: 30 additions & 3 deletions packages/wonder-blocks-core/src/components/__tests__/text.test.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import * as React from "react";
import {render} from "@testing-library/react";

import {render, screen} from "@testing-library/react";
import Text from "../text";

describe("Title", () => {
describe("Text", () => {
test("forwards the ref to the heading element", () => {
// Arrange
const ref: React.RefObject<HTMLSpanElement> = React.createRef();
Expand All @@ -14,4 +13,32 @@ describe("Title", () => {
// Assert
expect(ref.current).toBeInstanceOf(HTMLSpanElement);
});

test("applies style to the text wrapper", () => {
// Arrange
render(<Text style={{color: "red"}}>Text</Text>);

// Act
const wrapper = screen.getByText("Text");

// Assert
expect(wrapper).toHaveStyle({color: "red"});
});

test("appends the className prop to the element's class list", () => {
// Arrange
const className = "some-class";

// Act
render(
<Text style={{color: "red"}} className={className}>
Text
</Text>,
);

const wrapper = screen.getByText("Text");

// Assert
expect(wrapper).toHaveClass(className);
});
});
7 changes: 6 additions & 1 deletion packages/wonder-blocks-core/src/components/text.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,17 @@ const Text = React.forwardRef(function Text(
style,
]);

// Make sure we include the className from the parent component, if any.
const classNames = otherProps.className
? [otherProps.className, styleAttributes.className].join(" ")
: styleAttributes.className;

return (
// @ts-expect-error [FEI-5019] - TS2322 - Type '{ children: ReactNode; style: any; className: string; "data-test-id": string | undefined; tabIndex?: number | undefined; id?: string | undefined; "data-modal-launcher-portal"?: boolean | undefined; ... 69 more ...; onBlur?: ((e: FocusEvent<...>) => unknown) | undefined; }' is not assignable to type 'IntrinsicAttributes'.
<Tag
{...otherProps}
style={styleAttributes.style}
className={styleAttributes.className}
className={classNames}
data-test-id={testId}
ref={ref}
>
Expand Down

0 comments on commit 6df21f7

Please sign in to comment.