Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolves #528 - spread props on HeroDescription #529

Merged
merged 5 commits into from
Feb 5, 2024

Conversation

alexbuiltit
Copy link
Contributor

Summary

Minor change that resolves #528 - allowing users to add custom class names to the HeroDescription component

List of notable changes:

  • added {...rest} to HeroDescription to allow for custom class names to be used, this pattern is being used in HeroHeading, HeroImage etc.

What should reviewers focus on?

One thing to consider is that when a custom class name is being used it will remove the Hero-description styles, this is existing behaviour on the HeroHeading therefore isn't a regression.

Steps to test:

  1. Run Storybook
  2. Modify the default Hero.stories.tsx file and include a className on the HeroDescription component.
  3. Open the Hero story on storybook, inspect element and see class added to the hero description.

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

Copy link

changeset-bot bot commented Feb 1, 2024

🦋 Changeset detected

Latest commit: 63fc524

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

This PR includes changesets to release 6 packages
Name Type
@primer/react-brand Patch
@primer/brand-primitives Patch
@primer/brand-e2e Patch
@primer/brand-fonts Patch
@primer/brand-config Patch
@primer/brand-storybook Patch

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

.changeset/dry-news-dress.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@rezrah rezrah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making the update @alexbuiltit. The changes looks good. One minor suggestion for the change log, but otherwise happy to :shipit:

Co-authored-by: Rez <13340707+rezrah@users.noreply.github.com>
@alexbuiltit
Copy link
Contributor Author

Hey @rezrah I've approved your suggestion, is there anything else I need to do or best to leave it with you/the team for now?

@rezrah
Copy link
Collaborator

rezrah commented Feb 5, 2024

Hey @rezrah I've approved your suggestion, is there anything else I need to do or best to leave it with you/the team for now?

👍 Thanks for resolving this issue @alexbuiltit, and for contributing to Primer. I'll get this merged for you now.

@rezrah rezrah merged commit d00cf4e into primer:main Feb 5, 2024
11 of 15 checks passed
@primer-css primer-css mentioned this pull request Feb 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 [BUG] - className is missing on <Hero.Description>
2 participants