Skip to content

Conversation

@jonrohan
Copy link
Member

Closes https://github.com/github/primer/issues/4309
Closes https://github.com/github/primer/issues/4408

Changelog

New

Changed

Removed

Remove the CSS modules feature flag from the Dialog component

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

Copilot AI review requested due to automatic review settings April 24, 2025 22:05
@jonrohan jonrohan requested a review from a team as a code owner April 24, 2025 22:05
@jonrohan jonrohan requested a review from hectahertz April 24, 2025 22:05
@changeset-bot
Copy link

changeset-bot bot commented Apr 24, 2025

🦋 Changeset detected

Latest commit: 9c1e28a

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 github-actions bot added the staff Author is a staff member label Apr 24, 2025
Copy link
Contributor

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 CSS modules feature flag from the Dialog component by eliminating conditionals based on the feature flag and replacing styled component wrappers with BoxWithFallback components.

  • Removed toggleStyledComponent wrappers and the useFeatureFlag hook from both Dialog and DialogV1 components.
  • Updated tests and stories to remove the FeatureFlags wrapper.

Reviewed Changes

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

Show a summary per file
File Description
packages/react/src/DialogV1/Dialog.tsx Removed feature flag logic and updated DialogHeader to use a simpler API.
packages/react/src/DialogV1/Dialog.test.tsx Removed tests solely relying on feature flag enabled behavior.
packages/react/src/Dialog/Dialog.tsx Replaced styled component wrappers with BoxWithFallback and removed feature flag checks.
packages/react/src/Dialog/Dialog.test.tsx Removed feature flag test scenarios.
packages/react/src/Dialog/Dialog.dev.stories.tsx Removed FeatureFlags wrappers from stories.
.changeset/open-bananas-smile.md Changeset to document the minor release and removal of CSS modules feature flag.
Files not reviewed (1)
  • packages/react/src/DialogV1/Dialog.module.css: Language not supported
Comments suppressed due to low confidence (2)

packages/react/src/Dialog/Dialog.tsx:312

  • The removal of toggleStyledComponent and corresponding changes now directly assign data-width and data-height attributes. Please verify that the CSS and component styling consistently consume these attributes across all usage scenarios.
const dimensionProps = enabled ? {'data-width': width, 'data-height': height} : {width, height}

packages/react/src/DialogV1/Dialog.tsx:21

  • The DialogHeader component no longer accepts theme and backgroundColor props. Please ensure that consumers and documentation are updated to reflect this API change.
export type DialogHeaderProps = React.PropsWithChildren<HTMLAttributes<HTMLDivElement>> & SxProp

@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 Apr 24, 2025
@github-actions
Copy link
Contributor

👋 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
Copy link
Contributor

github-actions bot commented Apr 24, 2025

size-limit report 📦

Path Size
packages/react/dist/browser.esm.js 101.85 KB (-0.81% 🔽)
packages/react/dist/browser.umd.js 102.13 KB (-0.67% 🔽)

@github-actions github-actions bot requested a deployment to storybook-preview-5969 April 24, 2025 22:17 Abandoned
@primer-integration
Copy link

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

@primer-integration
Copy link

🟢 golden-jobs completed with status success.

@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 Apr 24, 2025
@jonrohan jonrohan enabled auto-merge April 24, 2025 23:02
@jonrohan jonrohan requested a review from joshblack April 24, 2025 23:02
@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 Apr 24, 2025
@github-actions
Copy link
Contributor

👋 Hi, there are new commits since the last successful integration test. We recommend running the integration workflow once more, unless you are sure the new changes do not affect github/github. Thanks!

@jonrohan jonrohan removed the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label Apr 24, 2025
@jonrohan jonrohan requested a review from francinelucca April 25, 2025 02:16
Copy link
Member

@francinelucca francinelucca left a comment

Choose a reason for hiding this comment

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

❇️

@jonrohan jonrohan added this pull request to the merge queue Apr 25, 2025
Merged via the queue into main with commit 1066419 Apr 25, 2025
46 checks passed
@jonrohan jonrohan deleted the css_modules_remove_flag/dialog branch April 25, 2025 02:56
@primer primer bot mentioned this pull request Apr 25, 2025
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 staff Author is a staff member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants