react icon indicating copy to clipboard operation
react copied to clipboard

Adds UnderlinePanels to `drafts/`

Open mperrotti opened this issue 1 year ago • 6 comments

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

This component is visually identical to UnderlineNav. The biggest differences are:

  • These are tabs as behave as such
  • It cannot be used as a controlled component
  • No responsive overflow menu (issue)

https://github.com/primer/react/assets/2313998/3d3e04a6-a0a3-4f0b-b0ef-7aaa4a443b48

Changelog

New

Adds UnderlinePanels component

Changed

Refactors UnderlineNav to share component styles with UnderlinePanels

Removed

Nothing removed

Rollout strategy

  • [ ] Patch release
  • [x] 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

This should more or less behave like PVC's UnderlinePanels

Merge checklist

  • [ ] Added/updated tests
  • [ ] Added/updated documentation
  • [x] Added/updated previews (Storybook)
  • [ ] Changes are SSR compatible
  • [x] Tested in Chrome
  • [x] 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)

mperrotti avatar May 01 '24 20:05 mperrotti

🦋 Changeset detected

Latest commit: 1cbc1624bc32738582a8062d8105d7fbbb721c07

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

changeset-bot[bot] avatar May 01 '24 20:05 changeset-bot[bot]

Added @keithamus as a reviewer since he's familiar with @github/tab-container-element, which does a lot of the heavy-lifting here.

mperrotti avatar May 02 '24 18:05 mperrotti

size-limit report 📦

Path Size
packages/react/dist/browser.esm.js 90.88 KB (+0.29% 🔺)
packages/react/dist/browser.umd.js 91.07 KB (+0.2% 🔺)

github-actions[bot] avatar May 03 '24 14:05 github-actions[bot]

I couldn't reproduce the tiny layout shift of UnderlineNav.Item's counters and icons in a real browser, so I updated UnderlineNav snapshots. If anybody can reproduce, I can make more changes and re-update snapshots.

mperrotti avatar May 03 '24 17:05 mperrotti

I'm not sure why the aXe tests are failing. The markup looks correct to me.

@owenniblock - did you encounter anything like this with TabPanels?

mperrotti avatar May 07 '24 22:05 mperrotti

I couldn't reproduce the tiny layout shift of UnderlineNav.Item's counters and icons in a real browser, so I updated UnderlineNav snapshots. If anybody can reproduce, I can make more changes and re-update snapshots.

I couldn't reproduce it either 😞

broccolinisoup avatar May 13 '24 02:05 broccolinisoup

Reviewers: I'll be out for 2 weeks. Please merge if this gets approved before I'm back.

mperrotti avatar May 21 '24 16:05 mperrotti

Snapshots need to be updated again since we changed text in stories.

mperrotti avatar May 21 '24 19:05 mperrotti

@broccolinisoup - how do I run the integration tests in dotcom?

mperrotti avatar Jun 12 '24 18:06 mperrotti

how do I run the integration tests in dotcom?

@mperrotti Here : https://github.com/github/primer-engineering/blob/main/how-we-work/testing-primer-react-pr-at-dotcom.md 😊 We also linked this in the PR template. It just makes sure that the primer/react PR doesn't break anything.

broccolinisoup avatar Jun 13 '24 09:06 broccolinisoup

@broccolinisoup - tests are failing for item-picker, but I don't think it has anything to do with the changes in this PR. Have you seen this when trying to do the release? Or do you think this truly is a problem with this PR?

mperrotti avatar Jun 14 '24 15:06 mperrotti