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

add classname prop support to PageHeader component and its children #4667

Merged

Conversation

ktravers
Copy link
Contributor

@ktravers ktravers commented Jun 12, 2024

Closes https://github.com/github/primer/issues/3331

This pull request introduces support for styling via CSS utility classes on the PageHeader component and its children by adding support for an optional className prop, allowing for more flexible customization without relying on Styled System props (sx). Full context: my team is currently in the process of deprecating our use of sx, so this change will allow us to safely incrementally replace sx props with className props.

Changelog

New

  • Adds optional className prop: All PageHeader components now accept a className prop. This prop is applied to the root element of each component.
  • Maintains backward compatibility: The addition of the className prop is optional and does not affect existing implementations that do not use this prop.
  • Updates TypeScript types: The TypeScript definitions for PageHeader components have been updated to include the new className prop, ensuring type safety for TypeScript users.
  • Updates documentation: The PageHeader component documentation has been updated to include information about the new className prop.

Changed

N/A

Removed

N/A

Rollout strategy

  • Patch release
  • Minor release
  • Major release; if selected, include a written rollout or migration plan
  • None; if selected, include a brief description as to why

Testing & Reviewing

Prerequisites

  1. Open this branch locally or in a codespace
  2. Run npm run setup to setup dependencies

Click testing

  1. Run npm start to start the Storybook server
  2. Navigate to the PageHeader component in Storybook
  3. Review examples

Automated testing

  1. Run npx playwright install --with-deps to install dependencies
  2. Run npx playwright test --grep @vrt to run visual regression tests
  3. Run npx playwright test --grep @art to run accessibility tests
  4. Review test results

Merge checklist

Copy link

changeset-bot bot commented Jun 12, 2024

🦋 Changeset detected

Latest commit: a44d6a0

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

This PR includes changesets to release 1 package
Name Type
@primer/react 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

@ktravers ktravers self-assigned this Jun 12, 2024
Copy link
Contributor

github-actions bot commented Jun 12, 2024

size-limit report 📦

Path Size
packages/react/dist/browser.esm.js 89.46 KB (0%)
packages/react/dist/browser.umd.js 89.73 KB (0%)

Copy link
Member

@broccolinisoup broccolinisoup left a comment

Choose a reason for hiding this comment

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

This is wonderful! 💖 Thank you so much for raising the PR and providing a great PR description!

@broccolinisoup
Copy link
Member

I am merging this so that I can include it in the current release! Thanks again 🙌🏻

@broccolinisoup broccolinisoup added this pull request to the merge queue Jun 13, 2024
Merged via the queue into main with commit e2a974f Jun 13, 2024
38 of 40 checks passed
@broccolinisoup broccolinisoup deleted the ktravers/add-classname-prop-to-page-header-and-children branch June 13, 2024 03:26
@primer primer bot mentioned this pull request Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants