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

Pagination: More precise type for renderItem #3275

Merged
merged 2 commits into from
Nov 1, 2024

Conversation

HalvorHaugan
Copy link
Contributor

@HalvorHaugan HalvorHaugan commented Oct 23, 2024

Description

The type for the props in renderItem should reflect what we actually send to it. This is in theory a breaking change, but seems like no one is using this prop anyways. And if anyone were, there's no reason why they would try to use any of the props that are now removed from the type (since they were not there in the first place).

Component Checklist 📝

  • JSDoc
  • Examples
  • Documentation
  • Storybook
  • Style mappings (@navikt/core/css/config/_mappings.js)
  • Component tokens (@navikt/core/css/tokens.json)
  • CSS class deprecations (@navikt/aksel-stylelint/src/deprecations.ts)
  • Exports (@navikt/core/react/src/index.ts and @navikt/core/react/package.json)
  • New component? CSS import (@navikt/core/css/index.css)
  • Breaking change? Update migration guide. Consider codemod.
  • Changeset (Format: <Component>: <gitmoji?> <Text>. E.g. "Button: ✨ Add feature xyz.")

Copy link

changeset-bot bot commented Oct 23, 2024

🦋 Changeset detected

Latest commit: e67fd5d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 7 packages
Name Type
@navikt/ds-react Patch
@navikt/ds-css Patch
@navikt/ds-tokens Patch
@navikt/ds-tailwind Patch
@navikt/aksel-icons Patch
@navikt/aksel Patch
@navikt/aksel-stylelint Patch

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

@@ -8,6 +8,17 @@ import PaginationItem, {
PaginationItemType,
} from "./PaginationItem";

interface RenderItemProps
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we export this?

Copy link
Contributor

@JulianNymark JulianNymark Oct 28, 2024

Choose a reason for hiding this comment

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

Only if it makes sense to use / pass down from outside 🤔
image
Man får jo typen inn når man bruker renderItem prop (forresten, visste ikke at vi hadde renderItem før nå 😅. Lagde nettopp min egen utrolig stygge pagination med box og borderWidth 😎). Det er kanskje ikke så mange brukere av denne propen? (er nice i de tilfellene man vil ha den)

Copy link
Contributor

Storybook demo

22f505170 | 91 komponenter | 144 stories

@HalvorHaugan HalvorHaugan merged commit 8cadfd0 into main Nov 1, 2024
4 checks passed
@HalvorHaugan HalvorHaugan deleted the pagination-renderitem-props branch November 1, 2024 07:49
@github-actions github-actions bot mentioned this pull request Nov 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants