-
Notifications
You must be signed in to change notification settings - Fork 6
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
Convert to TypeScript, rework support for React 18, Refresh latest logic from JotForm #6
base: main
Are you sure you want to change the base?
Conversation
…gic from JotForm.
@sbayd @mg-aceik Hey on both the main branch, as well as this PR we are getting a linting error when using this module: Type error: Could not find a declaration file for module 'jotform-react'. '/app/node_modules/jotform-react/dist/index.cjs.js' implicitly has an 'any' type. Wanted to flag this, thanks! |
@awc120 this PR isn't published to NPM yet so that issue isn't from this. To use this PR just copy the https://github.com/mg-aceik/jotform-react/blob/main/lib/index.tsx into your solution. |
can we publish this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason the lockfile is deleted? It should be regenerated and added to this pr. Otherwise, I think it looks good!
(edit) just realized your initial comment mentioned this. Should still get corrected prior to merge. I am not sure if this is maintained anymore though
@mg-aceik would it be possible to generate so we can release this update? |
@mg-aceik can we get this published? |
Someone else will need to take over the requested changes here, I'm not using Jotform anymore - switched to Paperform. |
I don't use pnpm so haven't checked in the lock file.
I also haven't checked the storybook, I was testing on a separate project.
Fixed #5.