Skip to content

remove unused contrast from Select component #6272

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

llastflowers
Copy link
Contributor

@llastflowers llastflowers commented Jul 2, 2025

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

Changelog

New

Changed

Removed

Instances of contrast in Select component, as it is unused.

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

Copy link

changeset-bot bot commented Jul 2, 2025

🦋 Changeset detected

Latest commit: ef24d65

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 Major

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

Copy link
Contributor

github-actions bot commented Jul 2, 2025

👋 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 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 Jul 2, 2025
Copy link
Contributor

github-actions bot commented Jul 2, 2025

size-limit report 📦

Path Size
packages/react/dist/browser.esm.js 92.38 KB (-0.02% 🔽)
packages/react/dist/browser.umd.js 92.46 KB (-0.04% 🔽)

@github-actions github-actions bot temporarily deployed to storybook-preview-6272 July 2, 2025 20:38 Inactive
@llastflowers llastflowers requested review from a team, hectahertz and lukasoppermann and removed request for a team July 2, 2025 20:45
@github-actions github-actions bot added integration-tests: passing Changes in this PR do NOT cause breaking changes in gh/gh and removed integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm labels Jul 2, 2025
@llastflowers llastflowers marked this pull request as ready for review July 3, 2025 16:48
@Copilot Copilot AI review requested due to automatic review settings July 3, 2025 16:48
Copilot

This comment was marked as outdated.

@llastflowers llastflowers requested a review from Copilot July 3, 2025 16:49
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR removes the unused contrast prop from the Select component to tidy up its API.

  • Eliminates contrast from the prop destructuring and forwarding in Select.tsx.
  • Adds a changeset to note the removal in a patch release.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
packages/react/src/Select/Select.tsx Remove contrast prop usage in Select component
.changeset/dirty-chairs-invent.md Add changeset entry for removing contrast
Comments suppressed due to low confidence (2)

packages/react/src/Select/Select.tsx:40

  • The contrast prop is still declared in the SelectProps interface (elsewhere) but has been removed from use—ensure it's also removed from the type definition and any public API docs to avoid exposing an unused prop.
      className,

packages/react/src/Select/Select.tsx:54

  • Since contrast is no longer forwarded to TextInputWrapper, remove its mention from component documentation, PropTables (e.g., Storybook), and any related tests to keep docs in sync with the API.
        block={block}

Copy link
Member

@joshblack joshblack left a comment

Choose a reason for hiding this comment

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

Just left a comment around how to sequence breaking changes, let me know what you think!

With this change, would we also want to Omit contrast from the StyledWrapperProps, as well, so it doesn't show up in the type info? (I think that's where it comes from but don't quote me on that lol)

This comment was marked as duplicate.

@github-actions github-actions bot added integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm and removed integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm labels Jul 3, 2025

This comment was marked as duplicate.

@github-actions 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 Jul 3, 2025
@github-actions github-actions bot removed the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label Jul 3, 2025
@github-actions github-actions bot requested a deployment to storybook-preview-6272 July 3, 2025 18:13 Abandoned

This comment was marked as duplicate.

@github-actions 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 Jul 3, 2025
@primer-integration
Copy link

👋 Hi from github/github! Your integration PR is ready: https://github.com/github/github/pull/388667

@github-actions github-actions bot removed the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label Jul 3, 2025
@github-actions github-actions bot requested a deployment to storybook-preview-6272 July 3, 2025 18:18 Abandoned
@github-actions 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 Jul 3, 2025

This comment was marked as duplicate.

@github-actions github-actions bot added integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm and removed integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm labels Jul 3, 2025

This comment was marked as resolved.

@primer-integration
Copy link

🟢 golden-jobs completed with status success.

@github-actions github-actions bot removed the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label Jul 3, 2025
@github-actions github-actions bot requested a deployment to storybook-preview-6272 July 3, 2025 18:35 Abandoned
@llastflowers llastflowers requested a review from joshblack July 3, 2025 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integration-tests: passing Changes in this PR do NOT cause breaking changes in gh/gh
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants