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

Restore/repair S3 image uploading in tinymce #4594

Merged
merged 3 commits into from
Jul 7, 2018

Conversation

Tathanen
Copy link
Contributor

@Tathanen Tathanen commented Mar 8, 2018

Description of changes

To get the S3 image uploading in HTML text entry working again:

  • Added back the uploadimage tinymce plugin
  • Properly required/defined some library code needed for S3FileType.js to function
    • I know this is a deprecated type but it's foundational in this functionality continuing to work. This is just a quick fix for now, a more thorough rearchitecting in the future is probably appropriate.

Related issues

keystonejs/keystone#4388, keystonejs/keystone#3529

Testing

There are like 54 failing tests and none of them have anything to do with this, but just including that per the form.

@Tathanen
Copy link
Contributor Author

Tathanen commented Mar 8, 2018

Of note: this is similar to the work done in keystonejs/keystone#4531, but also includes the necessary edits to make the S3 image uploading work, not just cloudinary.

@davidmckenzie
Copy link

Really hoping this could be merged soon - it's a blocking issue for me and I'd rather not deviate from master in production :)

@Noviny any chance we could get this merged?

@stennie
Copy link
Contributor

stennie commented May 29, 2018

Related: PR #4616 (add tinymce uploadimage plugin)

@stennie
Copy link
Contributor

stennie commented Jul 7, 2018

Thanks for the PR @Tathanen and apologies for the delay in feedback. With some effort we have tests passing in master once again and can start reviewing PRs more sanely.

This appears ok to merge, however I've created a related issue to update to the latest TinyMCE (#4699) which may introduce conflicts. If you're interested in helping with that, any assistance/advice would be appreciated.

Thanks,
Stennie

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

Successfully merging this pull request may close these issues.

3 participants