Skip to content

fix PopupProps type error #319

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

duniyalr
Copy link

@duniyalr duniyalr commented Aug 2, 2022

hi;
there is a type error when you want to pass a function instead a of ReactNode to Popup component.
this issue #315
i just changed the type of children in PopupProps to handle this.

@jfilipeferreira96
Copy link

Already tested and this will fix the typescript errors.

@@ -28,7 +28,7 @@ export interface PopupProps {
nested?: boolean;
defaultOpen?: boolean;
on?: EventType | EventType[];
children: React.ReactNode;
children: React.ReactNode | ((close: () => void, isClose?: boolean) => React.ReactNode);
Copy link

@DerPipo DerPipo Nov 30, 2022

Choose a reason for hiding this comment

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

Suggested change
children: React.ReactNode | ((close: () => void, isClose?: boolean) => React.ReactNode);
children: React.ReactNode | ((close: () => void, isOpen: boolean) => React.ReactNode);

Shouldn't it be called isOpen (instead of isClose)? And as you can see in line 306, the second param is always provided and cannot be undefined, so the ? is unnecessary.

But otherwise the changes look good and should get merged.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants