Add prop to PageHeader to allow a bottom border #5741

mperrotti · 2025-03-06T21:15:58Z

Relates to https://github.com/github/primer/issues/2837

Changelog

New

hasBorder prop on PageHeader

Changed

Removed

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

Merge checklist

Added/updated tests
Added/updated documentation
Added/updated previews (Storybook)
Changes are SSR compatible
Tested in Chrome
Tested in Firefox
Tested in Safari
Tested in Edge
(GitHub staff only) Integration tests pass at github/github (Learn more about how to run integration tests)

changeset-bot · 2025-03-06T21:16:01Z

🦋 Changeset detected

Latest commit: 18f09dd

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

github-actions · 2025-03-06T21:16:20Z

Uh oh! @mperrotti, at least one image you shared is missing helpful alt text. Check your pull request body to fix the following violations:

Images should have meaningful alternative text (alt text) at line 7

Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image.

Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.

🤖 Beep boop! This comment was added automatically by github/accessibility-alt-text-bot.

github-actions · 2025-03-06T21:16:28Z

👋 Hi, this pull request contains changes to the source code that github/github depends on. If you are GitHub staff, we recommend testing these changes with github/github using the integration workflow. Thanks!

github-actions · 2025-03-06T21:19:36Z

size-limit report 📦

Path	Size
packages/react/dist/browser.esm.js	105.57 KB (+0.09% 🔺)
packages/react/dist/browser.umd.js	105.94 KB (+0.09% 🔺)

packages/react/src/PageHeader/PageHeader.tsx

joshblack · 2025-03-07T02:18:26Z

packages/react/src/PageHeader/PageHeader.tsx

@@ -754,6 +762,7 @@ const Navigation: React.FC<React.PropsWithChildren<NavigationProps>> = ({
      aria-label={as === 'nav' ? ariaLabel : undefined}
      aria-labelledby={as === 'nav' ? ariaLabelledBy : undefined}
      className={clsx(enabled && classes.Navigation, className)}
+      data-component="PH_Navigation"


Suggested change

data-component="PH_Navigation"

data-component="PageHeader.Navigation"

One suggestion that could make it more clear what the component is when coming back to this later

PH_{Subcomponent} is the naming pattern we use for the other subcomponents. Do you still think we should change it?

I think we're kind of trying to move away from data-component actually.. what about just giving it a classname instead?

I agree, PageHeader.Navigation is more clear, but to keep it consistent I think PH_{Subcomponent} is fine too! Would be curious about the className route too 👀 Maybe we'll want to revisit in a different PR to adjust the existing data-* attributes.

lukasoppermann · 2025-03-07T09:21:18Z

@mperrotti somehow I can't add this comment to the code, but we need this: hasBorder={args.hasBorder} on the Playground story: https://github.com/primer/react/pull/5741/files#diff-38be7109ab257cb99d6f1950aa251d530e55a99c8705aaf94ff68f2d98c82897R195

lukasoppermann · 2025-03-07T09:23:18Z

Also it is buggy for those two cases:

I wonder if we should disable those cases, so that lines can not be added when a Navigation or description is enabled?

But for a normal text description it could be useful.

Co-authored-by: Josh Black <[email protected]>

mperrotti · 2025-03-10T12:51:09Z

@lukasoppermann - I thought I handled those cases already, but I must have broke something. I'll take another look.

…o mp/pageheader-hasBorder

…Border

emilybrick

Snapshots look good here & in storybook

I'm not sure if the naming pattern discussion is resolved, but story + snapshots lgtm 👍

TylerJDev

LGTM!

mperrotti added 2 commits March 6, 2025 16:11

adds prop to PageHeader to add a bottom border

de8e90d

adds changeset

47c50c7

mperrotti requested a review from a team as a code owner March 6, 2025 21:15

mperrotti requested a review from joshblack March 6, 2025 21:15

github-actions bot added the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label Mar 6, 2025

github-actions bot requested a deployment to storybook-preview-5741 March 6, 2025 21:20 Abandoned

mperrotti deployed to github-pages March 6, 2025 21:33 — with GitHub Actions View deployment

github-actions bot temporarily deployed to storybook-preview-5741 March 6, 2025 21:33 Inactive

joshblack reviewed Mar 7, 2025

View reviewed changes

primer deleted a comment from github-actions bot Mar 7, 2025

Update packages/react/src/PageHeader/PageHeader.tsx

7029c0f

Co-authored-by: Josh Black <[email protected]>

mperrotti deployed to github-pages March 10, 2025 13:03 — with GitHub Actions View deployment

github-actions bot temporarily deployed to storybook-preview-5741 March 10, 2025 13:03 Inactive

mperrotti added 3 commits March 10, 2025 10:34

got smarter about when we hide and show the bottom border

13f0797

Merge branch 'mp/pageheader-hasBorder' of github.com:primer/react int…

9b8fe10

…o mp/pageheader-hasBorder

Merge branch 'main' of github.com:primer/react into mp/pageheader-has…

3713e67

…Border

mperrotti deployed to github-pages March 10, 2025 14:48 — with GitHub Actions View deployment

github-actions bot temporarily deployed to storybook-preview-5741 March 10, 2025 14:49 Inactive

lukasoppermann mentioned this pull request Mar 11, 2025

Add Figma Code connect PageHeader #5757

Merged

13 tasks

Merge branch 'main' into mp/pageheader-hasBorder

5c11ae1

mperrotti deployed to github-pages March 11, 2025 21:02 — with GitHub Actions View deployment

github-actions bot temporarily deployed to storybook-preview-5741 March 11, 2025 21:02 Inactive

Merge branch 'main' into mp/pageheader-hasBorder

6705b40

primer bot requested a review from emilybrick March 18, 2025 21:03

github-actions bot removed the update snapshots label Mar 18, 2025

github-actions bot temporarily deployed to storybook-preview-5741 March 18, 2025 21:09 Inactive

primer bot deployed to github-pages March 18, 2025 21:23 View deployment

github-actions bot temporarily deployed to storybook-preview-5741 March 18, 2025 21:23 Inactive

mperrotti added the update snapshots label Mar 18, 2025

github-actions bot removed the update snapshots label Mar 18, 2025

primer bot deployed to github-pages March 19, 2025 14:53 View deployment

github-actions bot temporarily deployed to storybook-preview-5741 March 19, 2025 14:53 Inactive

emilybrick approved these changes Mar 19, 2025

View reviewed changes

mperrotti enabled auto-merge (squash) March 19, 2025 17:10

TylerJDev approved these changes Mar 19, 2025

View reviewed changes

Merge branch 'main' into mp/pageheader-hasBorder

52eecc8

mperrotti deployed to github-pages March 21, 2025 13:51 — with GitHub Actions View deployment

github-actions bot temporarily deployed to storybook-preview-5741 March 21, 2025 13:52 Inactive

mperrotti added the update snapshots label Mar 21, 2025

github-actions bot removed the update snapshots label Mar 21, 2025

mperrotti deployed to github-pages March 25, 2025 16:06 — with GitHub Actions View deployment

github-actions bot temporarily deployed to storybook-preview-5741 March 25, 2025 16:06 Inactive

Merge branch 'main' into mp/pageheader-hasBorder

18f09dd

mperrotti deployed to github-pages March 25, 2025 17:17 — with GitHub Actions View deployment

github-actions bot deployed to storybook-preview-5741 March 25, 2025 17:17 View deployment

TylerJDev disabled auto-merge March 25, 2025 17:31

TylerJDev added this pull request to the merge queue Mar 25, 2025

TylerJDev removed this pull request from the merge queue due to a manual request Mar 25, 2025

mperrotti added this pull request to the merge queue Mar 25, 2025

Merged via the queue into main with commit d32dd27 Mar 25, 2025
44 checks passed

mperrotti deleted the mp/pageheader-hasBorder branch March 25, 2025 17:44

primer bot mentioned this pull request Mar 25, 2025

Release tracking (37.17.0) #5802

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add prop to PageHeader to allow a bottom border #5741

Add prop to PageHeader to allow a bottom border #5741

Uh oh!

mperrotti commented Mar 6, 2025

Uh oh!

changeset-bot bot commented Mar 6, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Mar 6, 2025

Uh oh!

github-actions bot commented Mar 6, 2025

Uh oh!

github-actions bot commented Mar 6, 2025 •

edited

Loading

Uh oh!

Uh oh!

joshblack Mar 7, 2025

Uh oh!

mperrotti Mar 10, 2025

Uh oh!

langermank Mar 18, 2025

Uh oh!

TylerJDev Mar 19, 2025 •

edited

Loading

Uh oh!

lukasoppermann commented Mar 7, 2025

Uh oh!

lukasoppermann commented Mar 7, 2025

Uh oh!

mperrotti commented Mar 10, 2025

Uh oh!

emilybrick left a comment

Uh oh!

TylerJDev left a comment

Uh oh!

Uh oh!

Uh oh!

	data-component="PH_Navigation"
	data-component="PageHeader.Navigation"

Add prop to PageHeader to allow a bottom border #5741

Add prop to PageHeader to allow a bottom border #5741

Uh oh!

Conversation

mperrotti commented Mar 6, 2025

Changelog

New

Changed

Removed

Rollout strategy

Testing & Reviewing

Merge checklist

Uh oh!

changeset-bot bot commented Mar 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

github-actions bot commented Mar 6, 2025

Uh oh!

github-actions bot commented Mar 6, 2025

Uh oh!

github-actions bot commented Mar 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

size-limit report 📦

Uh oh!

Uh oh!

joshblack Mar 7, 2025

Choose a reason for hiding this comment

Uh oh!

mperrotti Mar 10, 2025

Choose a reason for hiding this comment

Uh oh!

langermank Mar 18, 2025

Choose a reason for hiding this comment

Uh oh!

TylerJDev Mar 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lukasoppermann commented Mar 7, 2025

Uh oh!

lukasoppermann commented Mar 7, 2025

Uh oh!

mperrotti commented Mar 10, 2025

Uh oh!

emilybrick left a comment

Choose a reason for hiding this comment

Uh oh!

TylerJDev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

changeset-bot bot commented Mar 6, 2025 •

edited

Loading

github-actions bot commented Mar 6, 2025 •

edited

Loading

TylerJDev Mar 19, 2025 •

edited

Loading