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

Update types.ts Refactor the openModal method type definition #153

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

Conversation

ramizqazi
Copy link

Ensure that params are required, so the function cannot be called without them as suggested in this issue #135

Ensure that params are required, so the function cannot be called without them.
@CharlesMangwa CharlesMangwa self-assigned this Aug 30, 2024
@CharlesMangwa CharlesMangwa self-requested a review August 30, 2024 11:59
Copy link
Member

@CharlesMangwa CharlesMangwa left a comment

Choose a reason for hiding this comment

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

hey @ramizqazi & thx for submitting this pr!

unfortunately, i cannot merge it as is for 2 reasons:

  1. not all openModal types were updated (see 1, 2 & 3)
  2. most importantly: your proposal unfortunately does not work when a modal doesn't expect any param. one would be forced to provide a 2nd argument anyway:
image

let me know if i can help you come up with a fix!

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.

2 participants