Integrate Icon component into Bento and Card

[joshfarrant]

Summary

Replace custom Icon implementations in Bento and Card components with new Icon component.

List of notable changes:

• Replace custom Icon implementations in Bento and Card components with new Icon component.
  • This also resolves a bug in the Bento component where the icon's default colour would be black, rather than the gray as it should have been. See here for the previous issue (variable should use base instead of brand).
• Update examples in docs to pass Icon component, rather than rendered element.
  • At one point I reference the not-yet-created Icon docs. These will be added in a subsequent PR.

What should reviewers focus on?

• Happy with any visual changes?
  • Note that the increased spacing between icon/label and heading in the Card component is an intentional change to align with the Figma Card.

Supporting resources (related issues, external links, etc):

• Closes [Modification Request] Update the icon in Bento.Item to appear the same as Card (with a background) #773

Contributor checklist:

• All new and existing CI checks pass
• Tests prove that the feature works and covers both happy and unhappy paths
• Any drop in coverage, breaking changes or regressions have been documented above
• New visual snapshots have been generated / updated for any UI changes
• All developer debugging and non-functional logging has been removed
• Related issues have been referenced in the PR description

Reviewer checklist:

• Check that pull request and proposed changes adhere to our contribution guidelines and code of conduct
• Check that tests prove the feature works and covers both happy and unhappy paths
• Check that there aren't other open Pull Requests for the same update/change

Screenshots:

Please try to provide before and after screenshots or videos

Before	After

[changeset-bot]

🦋 Changeset detected

Latest commit: eb51421

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages

Name	Type
@primer/brand-primitives	Minor
@primer/react-brand	Minor
@primer/brand-e2e	Minor
@primer/brand-fonts	Minor
@primer/brand-config	Minor
@primer/brand-storybook	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[github-actions]

🔍 Design token changes found

View CSS variable changes

- --brand-Icon-color-default: var(--base-color-scale-gray-5);
+ --brand-Icon-color-default: var(--brand-color-text-default);

[github-actions]

🟢 No visual differences found

Our visual comparison tests did not find any differences in the UI.

[rezrah]

Does Icon still support passing the instantiated component, or only reference? I.e. if user wanted to customize values here, can they still do it?

[joshfarrant]

No, that was removed by design after speaking with @danielguillan to avoid situations like this:

<Card.Icon icon={<CopilotIcon size={20} />} size={44} />

Since Card.Icon forwards all of its props to the provided icon, you could easily end up in these awkward situations where two props clash.

Do you think this should be called out as a breaking change in the changeset?

[rezrah]

Thanks for this example. Makes sense 👍

This is a breaking change if we don't allow instantiated JSX anymore, yeah. Unless there's a really important reason for it, we should continue to support both ways, as we have many instances in dotcom and elsewhere that use that approach and we can more easily work around it on our side than the users. In many of those instances, the user doesn't apply size too, so this is less of a risk IMO.

E.g.

Suggestion:

Update our docs and examples to show the reference approach (happy path), and ensure that instantiated JSX can continue to be passed for continuity. How does that sound?

[rezrah]

Update for posterity.. I've pushed the changes ☝️ in this commit

• passing JSX is now working
• removed breaking change notice
• added a new story to track future regressions

[rezrah]

Is props doing something specific here or can we keep it the same as the other stories / the same as before?

[joshfarrant]

props includes the classNames, without it the Icon styles won't be applied to the svg

[rezrah]

I'm guessing this was needed because we only accepted function references? What would happen if someone forgot to spread the props?

For now, I've rolled this back to allow passing the SVG directly as a prop. We can figure out if the props argument is still needed when you're back.

[danielguillan]

Looking great! It just needs the adjustment to the default icon color we discussed in Slack.