-
Notifications
You must be signed in to change notification settings - Fork 605
chore(TooltipV2): Remove the CSS modules feature flag from the TooltipV2 component #5872
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
Conversation
🦋 Changeset detectedLatest commit: 142297d The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
👋 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! |
size-limit report 📦
|
Co-authored-by: Jon Rohan <[email protected]>
…arStack component (#5871)
Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Hector Garcia <[email protected]> Co-authored-by: Josh Black <[email protected]>
… from the AvatarStack component" (#5879)
Co-authored-by: Marie Lucca <[email protected]> Co-authored-by: Marie Lucca <[email protected]>
Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…he ActionList.Heading component (#5900)
#5886) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Marie Lucca <[email protected]>
Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Marie Lucca <[email protected]>
…contexts (#5866) Co-authored-by: Marie Lucca <[email protected]>
👋 Hi from github/github! Your integration PR is ready: https://github.com/github/github/pull/374797 |
🟢 golden-jobs completed with status |
There was a problem hiding this 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 TooltipV2 component, streamlining the tooltip styling logic. Key changes include:
- Removal of styled-components-based logic and the feature flag.
- Replacement of the StyledTooltip with a new BaseComponent using toggleSxComponent.
- Update of className usage to always include the Tooltip CSS module.
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.
File | Description |
---|---|
packages/react/src/TooltipV2/Tooltip.tsx | Removed CSS modules feature flag imports and conditional styling; replaced with BaseComponent |
.changeset/petite-women-notice.md | Added changeset notice for removing the CSS modules feature flag from TooltipV2 |
Files not reviewed (1)
- packages/react/src/tests/snapshots/TextInput.test.tsx.snap: Language not supported
Comments suppressed due to low confidence (2)
packages/react/src/TooltipV2/Tooltip.tsx:84
- [nitpick] Consider renaming 'BaseComponent' to a more descriptive name (e.g. 'TooltipContainer') to clearly indicate its role in the TooltipV2 component.
const BaseComponent = toggleSxComponent('span') as React.ComponentType< SxProp & React.HTMLAttributes<HTMLElement> & React.RefAttributes<HTMLSpanElement> >
packages/react/src/TooltipV2/Tooltip.tsx:291
- Ensure that the removal of the conditional CSS modules feature flag and the direct application of the Tooltip CSS class are fully covered by tests to prevent styling regressions across browsers.
<BaseComponent className={clsx(className, classes.Tooltip)} ref={tooltipElRef} data-direction={calculatedDirection} {...rest}>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✨
👋 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! |
Closes https://github.com/github/primer/issues/4296
Changelog
New
Changed
Removed
Remove the CSS modules feature flag from the TooltipV2 component
Rollout strategy
Testing & Reviewing
Merge checklist