Skip to content
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

feat(pagination): add forwardedRef for pagination container div #10239

Merged

Conversation

kevinsperrine
Copy link
Contributor

@kevinsperrine kevinsperrine commented Dec 7, 2021

Closes #10240

This PR adds a forwardedRef prop to the Pagination component. This allows us to check the size of the component and adjust styles as necessary in different circumstances.

Changelog

New

  • add forwardedRef prop to Pagination component. This allows us to check the getBoundingClientRect of the pagination container and change classes or styles as necessary. We can achieve this via wrapping the Pagination with an extra div, but it would be nicer to avoid the extra markup.

Testing / Reviewing

Pagination should be unaffected in stories and the new test should pass confirming the presence of the ref.

@kevinsperrine kevinsperrine requested a review from a team as a code owner December 7, 2021 00:44
@netlify
Copy link

netlify bot commented Dec 7, 2021

✔️ Deploy Preview for carbon-react-next ready!

🔨 Explore the source changes: aa3921c

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-react-next/deploys/61f826d042c924000890b0aa

😎 Browse the preview: https://deploy-preview-10239--carbon-react-next.netlify.app

@netlify
Copy link

netlify bot commented Dec 7, 2021

❌ Deploy Preview for carbon-components-react failed.
Built without sensitive environment variables

🔨 Explore the source changes: 23c4d8f

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-components-react/deploys/61aeae6e9ba1aa0007dffe1e

@netlify
Copy link

netlify bot commented Dec 7, 2021

✔️ Deploy Preview for carbon-elements ready!

🔨 Explore the source changes: 23c4d8f

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-elements/deploys/61aeae6e35001d0008b20cf8

😎 Browse the preview: https://deploy-preview-10239--carbon-elements.netlify.app

@netlify
Copy link

netlify bot commented Dec 7, 2021

❌ Deploy Preview for carbon-components-react failed.
Built without sensitive environment variables

🔨 Explore the source changes: ffe816e

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-components-react/deploys/61aeb1f1a70ec70007800c64

@netlify
Copy link

netlify bot commented Dec 7, 2021

✔️ Deploy Preview for carbon-elements ready!

🔨 Explore the source changes: ffe816e

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-elements/deploys/61aeb1f14139e100070853d2

😎 Browse the preview: https://deploy-preview-10239--carbon-elements.netlify.app

@netlify
Copy link

netlify bot commented Dec 7, 2021

❌ Deploy Preview for carbon-components-react failed.
Built without sensitive environment variables

🔨 Explore the source changes: 3f477bb

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-components-react/deploys/61aeb534eabf9e0007277fa4

@netlify
Copy link

netlify bot commented Dec 7, 2021

✔️ Deploy Preview for carbon-components-react ready!

🔨 Explore the source changes: aa3921c

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-components-react/deploys/61f826d0f390b700079b3851

😎 Browse the preview: https://deploy-preview-10239--carbon-components-react.netlify.app

@netlify
Copy link

netlify bot commented Dec 7, 2021

✔️ Deploy Preview for carbon-elements ready!

🔨 Explore the source changes: aa3921c

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-elements/deploys/61f826d042c924000890b0a8

😎 Browse the preview: https://deploy-preview-10239--carbon-elements.netlify.app

@kevinsperrine kevinsperrine requested a review from a team as a code owner December 7, 2021 03:04
@@ -312,7 +321,7 @@ export default class Pagination extends Component {
const pageSizes = mapPageSizesToObject(_pageSizes);

return (
<div className={classNames} {...other}>
<div className={classNames} ref={forwardedRef} {...other}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm.. thoughts on doing something like rest.ref rather than adding a whole new prop for it in v10, and having to deprecate it in v11 when it's no longer needed?
DatePicker class component did this with inline to not have to introduce it as a prop:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call. That is definitely cleaner. I will adjust.

@jnm2377
Copy link
Contributor

jnm2377 commented Jan 10, 2022

hi @kevinsperrine just wanted to check in on this and see if it's something you're still working on?

@kevinsperrine
Copy link
Contributor Author

hi @kevinsperrine just wanted to check in on this and see if it's something you're still working on?

Hi @jnm2377, Sorry, I forgot about this after the holidays. Yes', I'll adjust and push up the changes today.

@jnm2377 jnm2377 merged commit 430693f into carbon-design-system:main Jan 31, 2022
@jnm2377 jnm2377 mentioned this pull request Feb 10, 2022
22 tasks
kennylam added a commit to kennylam/carbon that referenced this pull request Jul 30, 2024
### Description

This upgrades the project to use Node 18.

**Note:** OpenSSL 3.0 is included as of Node 17, which can cause errors in packages within the application that haven't yet been upgraded. To prevent this, the option `--openssl-legacy-provider` is used with certain script commands. In Carbon for IBM.com, Webpack 4 is the main breaking point. Once it is upgraded to Webpack 5 the errors should be resolved. If not, a further audit/upgrade of dependencies will be required.

Read more about the OpenSSL upgrade [here](https://medium.com/the-node-js-collection/node-js-17-is-here-8dba1e14e382).

### Changelog

**New**

- {{new thing}}

**Changed**

- project now uses Node 18
- changed various import paths to use `internal/vendor`
- upgrade various deps as required by Node 18

**Removed**

- unneeded test assertions and Codesandbox examples

<!-- React and Web Component deploy previews are enabled by default. -->
<!-- To enable additional available deploy previews, apply the following -->
<!-- labels for the corresponding package: -->
<!-- *** "test: e2e": Codesandbox examples and e2e integration tests -->
<!-- *** "package: services": Services -->
<!-- *** "package: utilities": Utilities -->
<!-- *** "RTL": React / Web Components (RTL) -->
<!-- *** "feature flag": React / Web Components (experimental) -->
kennylam added a commit to kennylam/carbon that referenced this pull request Jul 30, 2024
…-design-system#10253)

* chore(deps): upgrade Node 18 (carbon-design-system#10239)

This upgrades the project to use Node 18.

**Note:** OpenSSL 3.0 is included as of Node 17, which can cause errors in packages within the application that haven't yet been upgraded. To prevent this, the option `--openssl-legacy-provider` is used with certain script commands. In Carbon for IBM.com, Webpack 4 is the main breaking point. Once it is upgraded to Webpack 5 the errors should be resolved. If not, a further audit/upgrade of dependencies will be required.

Read more about the OpenSSL upgrade [here](https://medium.com/the-node-js-collection/node-js-17-is-here-8dba1e14e382).

**New**

- {{new thing}}

**Changed**

- project now uses Node 18
- changed various import paths to use `internal/vendor`
- upgrade various deps as required by Node 18

**Removed**

- unneeded test assertions and Codesandbox examples

<!-- React and Web Component deploy previews are enabled by default. -->
<!-- To enable additional available deploy previews, apply the following -->
<!-- labels for the corresponding package: -->
<!-- *** "test: e2e": Codesandbox examples and e2e integration tests -->
<!-- *** "package: services": Services -->
<!-- *** "package: utilities": Utilities -->
<!-- *** "RTL": React / Web Components (RTL) -->
<!-- *** "feature flag": React / Web Components (experimental) -->

* fix(filter-panel): missing modal imports (carbon-design-system#10204)

### Related Ticket(s)

carbon-design-system#10200 
### Description

in small breakpoints the modal header and close button were missing imports

### Changelog

**New**

- added modal imports

<!-- React and Web Component deploy previews are enabled by default. -->
<!-- To enable additional available deploy previews, apply the following -->
<!-- labels for the corresponding package: -->
<!-- *** "test: e2e": Codesandbox examples and e2e integration tests -->
<!-- *** "package: services": Services -->
<!-- *** "package: utilities": Utilities -->
<!-- *** "RTL": React / Web Components (RTL) -->
<!-- *** "feature flag": React / Web Components (experimental) -->

* chore(storybook): update cwc urls (carbon-design-system#10207)

Update Storybook URLs to monorepo `carbon-web-components` package

**New**

- {{new thing}}

**Changed**

- change Storybook repo urls to monorepo package

**Removed**

- unused deploy script

<!-- React and Web Component deploy previews are enabled by default. -->
<!-- To enable additional available deploy previews, apply the following -->
<!-- labels for the corresponding package: -->
<!-- *** "test: e2e": Codesandbox examples and e2e integration tests -->
<!-- *** "package: services": Services -->
<!-- *** "package: utilities": Utilities -->
<!-- *** "RTL": React / Web Components (RTL) -->
<!-- *** "feature flag": React / Web Components (experimental) -->

* fix(rolloup): point to cwc internal vendor components for custom babel plugin (carbon-design-system#10209)

N/A

Edit roll up config for custom babel plugin that prevents the `customElements.define()` from happening for the cwc components - look for the internal vendor carbon web component components instead to prevent the custom registry error

**Changed**

- point to cwc internal vendor components for custom babel plugin

<!-- React and Web Component deploy previews are enabled by default. -->
<!-- To enable additional available deploy previews, apply the following -->
<!-- labels for the corresponding package: -->
<!-- *** "test: e2e": Codesandbox examples and e2e integration tests -->
<!-- *** "package: services": Services -->
<!-- *** "package: utilities": Utilities -->
<!-- *** "RTL": React / Web Components (RTL) -->
<!-- *** "feature flag": React / Web Components (experimental) -->

* chore(storybook): fix storybook:react commands

* chore(merge): resolve conflicts

* chore(snapshots): fixes updateSnapshot script; new snapshots generated with new prefix

---------

Co-authored-by: Ariella Gilmore <[email protected]>
Co-authored-by: Anna Wen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request]: add forward ref to Pagination container
3 participants