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

feat #55 enable upload widget instance methods #78

Merged
merged 12 commits into from
Jan 16, 2024
Merged

feat #55 enable upload widget instance methods #78

merged 12 commits into from
Jan 16, 2024

Conversation

matiasfha
Copy link
Contributor

@matiasfha matiasfha commented Dec 26, 2023

Description

Updates the CldUploadWidtet component to enable all of the cloudinary instance methods.

Also updates de CldUploadWidget documentation

Issue Ticket Number

Fixes #55
Fixes #67
Fixes #69
Fixes #75
Fixes #76

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Fix or improve the documentation
  • This change requires a documentation update

Checklist

  • I have followed the contributing guidelines of this project as mentioned in CONTRIBUTING.md
  • I have created an issue ticket for this PR
  • I have checked to ensure there aren't other open Pull Requests for the same update/change?
  • I have performed a self-review of my own code
  • I have run tests locally to ensure they all pass
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes needed to the documentation

Copy link

vercel bot commented Dec 26, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
svelte-cloudinary ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 16, 2024 5:10pm

@colbyfayock
Copy link
Contributor

Getting an undefined on this column

image

can we tidy up the spacing around the code blocks? you may be able to trim that in the component. also can we add a copy button for the snippet? the one in Next is available upon hover of the code block

image image image

getting this post message error. i saw something similar in Next.js, are we tracking this here? is this new? the issue there was trying to pass in non-serializable data through the instance methods: cloudinary-community/next-cloudinary#261

image

@matiasfha
Copy link
Contributor Author

Where and when that error shows up?

@colbyfayock
Copy link
Contributor

@matiasfha it shows up when using the upload widget examples in the docs

@matiasfha
Copy link
Contributor Author

Really weird stuff because I can't see that error on production nor local

}

if (typeof widget?.[method] === 'function') {
const validOptions = options.filter((option) => typeof option !== 'undefined').filter(option => !(option instanceof PointerEvent));
Copy link
Contributor

Choose a reason for hiding this comment

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

do you think the specificity of filtering on PointerEvent will be enough safeguard for the future? wondering if it's going to just wind up back here again 🤔 it may be worth checking what's being sent through thats not valid and make sure it doesnt get here in the first place

here's the latest in Next Cloudinary that handles this: https://github.com/cloudinary-community/next-cloudinary/blob/main/next-cloudinary/src/components/CldUploadWidget/CldUploadWidget.tsx#L166

the options need to be serializable so something must be getting through somehow thats not

Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe we can add something like

const validOptions = JSON.parse(JSON.stringify(options))

That will remove all non-serializable data

@colbyfayock
Copy link
Contributor

not able to produce the postMessage error anymore but left a comment on the solution

@matiasfha matiasfha merged commit 092e814 into main Jan 16, 2024
6 checks passed
Copy link
Contributor

🎉 This PR is included in version 1.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@ghostdevv ghostdevv deleted the feat-55 branch March 11, 2024 01:54
@ghostdevv ghostdevv restored the feat-55 branch March 11, 2024 01:54
@ghostdevv ghostdevv deleted the feat-55 branch May 31, 2024 23:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants