search

dequelabs / cauldron

https://cauldron.dequelabs.com/
Mozilla Public License 2.0
91 stars 21 forks source link

Pagination statusLabel prop should be more restrictive #958

Closed thuey closed 1 year ago

thuey commented 1 year ago

Currently, the Pagination statusLabel prop accepts ReactNode: https://github.com/dequelabs/cauldron/blob/655da6e04d8fd20a817a8325c5c2f2ce7aa93449/packages/react/src/components/Pagination/Pagination.tsx#L12

This is more permissive than it should be. It's important that the label contain valid content and not null | undefined | boolean. We should update the label prop type to be ContentNode: https://github.com/dequelabs/cauldron/blob/655da6e04d8fd20a817a8325c5c2f2ce7aa93449/packages/react/src/types.ts#L9-L18

Note: This would be a breaking change

Bracciata commented 1 year ago

Do we have a method for handling breaking changes?

thuey commented 1 year ago

Do we have a method for handling breaking changes?

@Bracciata See: https://github.com/dequelabs/cauldron/blob/develop/CONTRIBUTING.md#breaking-changes

You will need to include BREAKING CHANGE: ... In the commit description when merging the PR. See also: https://www.conventionalcommits.org/en/v1.0.0/#commit-message-with-description-and-breaking-change-footer