Skip to content

Commit

Permalink
fix(Icon): Use BEM for Icon modifier classes (#364)
Browse files Browse the repository at this point in the history
## Process
- I've confirmed that upgrading to the latest `cfpb-icons` library is
the source of the breakage, causing the `updating` icons to no longer be
animated
- I've confirmed that other icon modifiers (ex. [Success alert icon
shown in
green](https://deploy-preview-364--cfpb-design-system-react.netlify.app/?path=/story/components-draft-alerts--success))
seem to be working as expected.

## How to test this PR

1. Visit
https://deploy-preview-364--cfpb-design-system-react.netlify.app/?path=/story/components-draft-icons--status-icons
1. Confirm that the `updating` icon is animated
  • Loading branch information
meissadia authored Jun 7, 2024
1 parent 5498b42 commit 49cadaa
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 7 deletions.
2 changes: 1 addition & 1 deletion src/components/Alert/Alert.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ describe('<Alert />', () => {
// Icon is displayed: External link
const externalIcon = await within(linkTwo).findByRole('img');
expect(externalIcon).toBeInTheDocument();
expect(externalIcon).toHaveClass('cf-icon-svg__external-link');
expect(externalIcon).toHaveClass('cf-icon-svg--external-link');
});

it('renders field-level alerts', async () => {
Expand Down
10 changes: 5 additions & 5 deletions src/components/Icon/Icon.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ describe('Icon', () => {

// Need to wait for icon to load
const icon = await screen.findByRole(/img/i);
expect(icon.getAttribute('class')).toMatch(`cf-icon-svg__${name}`);
expect(icon.getAttribute('class')).toMatch(`cf-icon-svg--${name}`);
});

it('Renders a round icon', async () => {
Expand All @@ -20,7 +20,7 @@ describe('Icon', () => {

// Need to wait for icon to load
const icon = await screen.findByRole(/img/i);
expect(icon.getAttribute('class')).toMatch(`cf-icon-svg__${name}`);
expect(icon.getAttribute('class')).toMatch(`cf-icon-svg--${name}`);
});

it('Renders a square icon', async () => {
Expand All @@ -30,7 +30,7 @@ describe('Icon', () => {

// Need to wait for icon to load
const icon = await screen.findByRole(/img/i);
expect(icon.getAttribute('class')).toMatch(`cf-icon-svg__${name}`);
expect(icon.getAttribute('class')).toMatch(`cf-icon-svg--${name}`);
});

it('Renders an open number icon', async () => {
Expand All @@ -40,7 +40,7 @@ describe('Icon', () => {

// Need to wait for icon to load
const icon = await screen.findByRole(/img/i);
expect(icon.getAttribute('class')).toMatch(`cf-icon-svg__${name}-open`);
expect(icon.getAttribute('class')).toMatch(`cf-icon-svg--${name}-open`);
});

it('Renders a closed number icon', async () => {
Expand All @@ -50,7 +50,7 @@ describe('Icon', () => {

// Need to wait for icon to load
const icon = await screen.findByRole(/img/i);
expect(icon.getAttribute('class')).toMatch(`cf-icon-svg__${name}-closed`);
expect(icon.getAttribute('class')).toMatch(`cf-icon-svg--${name}-closed`);
});

it('Provides message for unknown icons', async () => {
Expand Down
2 changes: 1 addition & 1 deletion src/components/Icon/Icon.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ export const Icon = ({
}: IconProperties): JSX.Element | null => {
const shapeModifier = getShapeModifier(name, withBg);
const fileName = `${name}${shapeModifier}`;
const classes = ['cf-icon-svg', `cf-icon-svg__${fileName}`];
const classes = ['cf-icon-svg', `cf-icon-svg--${fileName}`];

const icon = useIconSvg(fileName);

Expand Down

0 comments on commit 49cadaa

Please sign in to comment.