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

[popover2] fix(Popover2): renderTarget typedef regression #5907

Merged
merged 3 commits into from
Jan 31, 2023

Conversation

adidahiya
Copy link
Contributor

Fixes #5889

Checklist

  • Includes tests
  • Update documentation

Changes proposed in this pull request:

  • Use AND & type union instead of OR | to allow access to common props passed to renderTarget
  • Add type param constraints to Tooltip2 (this should fix the linked issue)

Reviewers should focus on:

No regressions, and the new unit test is appropriate

@adidahiya
Copy link
Contributor Author

add default type params

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

@adidahiya
Copy link
Contributor Author

clean up types

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

@@ -568,13 +568,7 @@ export const DateInput2: React.FC<DateInput2Props> = React.memo(function _DateIn

// We use the renderTarget API to flatten the rendered DOM and make it easier to implement features like the "fill" prop.
const renderTarget = React.useCallback(
// N.B. pull out `defaultValue` so that it's not forwarded to the DOM.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note that this is another benefit of the type changes in #5802 -- we no longer have to deal with extraneous props which are not actually passed to the target renderer, but were presumed to exist in the previous type definitions 👍🏽

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.

New type for IPopover2SharedProps.renderTarget creates ambiguous type where properties are resolved as unknown
1 participant