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

Fix error messages for custom uploaders #14029

Merged
merged 4 commits into from
Dec 14, 2023
Merged

Conversation

timkelty
Copy link
Contributor

Description

This PR broke things for custom uploaders (eg Cloud), as data is undefined in that context.

This is because we need to support the ancient blueimp/jQuery-File-Upload events, as well as standard JS events for alternate uploaders.

So, we need account for data being null, as well as event being an object with different props.

To try and clarify this a bit, I added a default null value for data.

@i-just I also used data.files rather than getting it from the FormData like you were, as that seemed more straightforward. If there is a reason not to use that let me know.

Related issues

Copy link
Contributor

@i-just i-just left a comment

Choose a reason for hiding this comment

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

Thanks @timkelty!

I tested your solution, and it works like my previous one in terms of plain cms uploads. data.files seems more straightforward, and I can’t think of a reason not to use it.

I guess the same approach should be used in the 3 other places, too, so I pushed changes for the following files:

  • elements/Asset.php (lines: 1485-1558) => triggered when you’re editing an individual asset and use the “Replace file” button
  • web/assets/cp/AssetSelectInput.js => triggered when you use direct upload in an Assets field (“Upload files” button)
  • web/assets/cp/ImageUpload.js => triggered when you upload a photo to a user profile

Any chance you can test this against the cloud?

@timkelty
Copy link
Contributor Author

@i-just your additions look 💯, but i'll give it a spin in a cloud project to make sure.

@timkelty
Copy link
Contributor Author

@i-just tested on cloud and looks good!

@brandonkelly brandonkelly merged commit 9849481 into develop Dec 14, 2023
@brandonkelly brandonkelly deleted the fix-upload-error-message branch December 14, 2023 12:37
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.

3 participants