-
Notifications
You must be signed in to change notification settings - Fork 615
chore(DataTable.Pagination): Convert DataTable.Pagination to CSS modules #6273
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
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 421bad1 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 📦
|
👋 Hi from github/github! Your integration PR is ready: https://github.com/github/github/pull/388357 |
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 converts the styling of the DataTable.Pagination
component (and the internal ButtonReset
component) from styled-components to CSS modules.
- Replaces the styled-components-based
ButtonReset
with a functional component that applies a CSS module andclsx
. - Migrates all pagination-related styles in
DataTable.Pagination
to a CSS module, replacingStyledPagination
and scoped class names. - Adds a changeset entry for a minor release.
Reviewed Changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
packages/react/src/internal/components/ButtonReset.tsx | Replaced styled-components with ButtonReset.module.css and a functional component. |
packages/react/src/DataTable/Pagination.tsx | Swapped out StyledPagination and inline styles for Pagination.module.css + clsx . |
.changeset/polite-pandas-tie.md | Added changeset marking the minor version bump for this chore. |
Files not reviewed (2)
- packages/react/src/DataTable/Pagination.module.css: Language not supported
- packages/react/src/internal/components/ButtonReset.module.css: Language not supported
Comments suppressed due to low confidence (1)
packages/react/src/internal/components/ButtonReset.tsx:11
- Missing import of React for using
React.PropsWithChildren
andReact.ButtonHTMLAttributes
. Please addimport type React from 'react'
at the top.
}: React.PropsWithChildren & React.ButtonHTMLAttributes<HTMLButtonElement>) => {
🟢 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.
Just one suggestion for the ButtonReset but otherwise LGTM 👍
border-end-end-radius: var(--borderRadius-medium); | ||
} | ||
|
||
@media ((max-width: calc(768px - 0.02px))) { |
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.
Completely separate from the refactor, would this be something we would ever want a container query for now that we can use them or is this best tied to the viewport range? (Was just curious)
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.
Yes I think this was a bit verbose and would love either media query variables here or container queries.
Co-authored-by: Josh Black <joshblack@github.com>
👋 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! |
👋 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! |
👋 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! |
👋 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! |
This converts the
DataTable.Pagination
component to CSS modules. This was overlooked in the original migration.Changelog
New
Changed
Converts styled-component styling to CSS modules.
Removed
Rollout strategy
Testing & Reviewing
Merge checklist