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

chore: upgrade xblock-utils to 3.2.0 to support TinyMCE v5 #34149

Conversation

Anas12091101
Copy link
Contributor

@Anas12091101 Anas12091101 commented Jan 31, 2024

Description

This PR upgrades xblock-utils to version 3.2.0 to support TinyMCE v5 in the Palm release. The current version of xblock-utils is 3.0.0, configured for TinyMCE v4. This configuration causes an error after the upgrading of TinyMCE from v4.0.20 to v5.5.1 in this PR: #30335

Useful information to include:

  • Which edX user roles will this change impact? Learner and Course Author using TinyMCE WYSIWYG editor in palm.
  • Screenshots:
    Before:
Screenshot 2024-01-31 at 3 22 45 PM After: Screenshot 2024-01-31 at 3 04 30 PM

Supporting information

Issue: mitodl/edx-sga#346
Discussion: https://discuss.openedx.org/t/tinymce-problem-in-edx-sga-problems-modern-theme-not-found/11148
xblock-util v3.2.0 TinyMCE changes PR: openedx-unsupported/xblock-utils#207

Testing instructions

  • Configure edx-sga (if not already) in your edx-platform palm release.
  • Create an edx-sga xblock using the instructions in the edx-sga repo.
  • Click Edit and scroll down to the Solution setting.
  • Ensure that the WYSIWYG editor is functioning properly.

Deadline

None

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Jan 31, 2024
@openedx-webhooks
Copy link

Thanks for the pull request, @Anas12091101! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

@mphilbrick211 mphilbrick211 added the needs test run Author's first PR to this repository, awaiting test authorization from Axim label Jan 31, 2024
@mphilbrick211 mphilbrick211 removed the needs test run Author's first PR to this repository, awaiting test authorization from Axim label Feb 9, 2024
@mphilbrick211 mphilbrick211 added the waiting for eng review PR is ready for review. Review and merge it, or suggest changes. label Feb 21, 2024
@mariajgrimaldi
Copy link
Member

Hi there, @Anas12091101; since there will not be another Palm release according to the Open edX release schedule, I'll close this PR in the name of BTR. You can open a backport PR to the Quince branch to include these changes in the next release if you haven't already or wait until Redwood. Thanks!

@Anas12091101
Copy link
Contributor Author

@mariajgrimaldi, this issue is only present in the Palm release. When TinyMCE was upgraded to version 5 in the Palm release #30335, thexblock-utils version was not updated to one that supports TinyMCE v5. Consequently, xblocks or plugins using xblock-utils for rendering a WYSIWYG editor in Palm are encountering this issue.

CC: @pdpinch @arslanashraf7

@mariajgrimaldi
Copy link
Member

@Anas12091101: I understand. Now, I closed this issue because, according to the Open edX release schedule, there will not be another Palm release. So, I'll take this conversation to the BTR Slack channel to hear what other folks think.

@Anas12091101
Copy link
Contributor Author

@mariajgrimaldi any updates on this?

@mariajgrimaldi mariajgrimaldi reopened this Apr 1, 2024
@mariajgrimaldi
Copy link
Member

mariajgrimaldi commented Apr 1, 2024

Here's the thread I opened: https://openedx.slack.com/archives/C049JQZFR5E/p1710851198308489. So, we won't have a new palm release but you folks could use palm.master in your deployments.

I'll try finding someone to review/merge this, thanks!

@Anas12091101
Copy link
Contributor Author

thanks

@DonatoBD
Copy link

DonatoBD commented Apr 2, 2024

Hi @Anas12091101 and @mariajgrimaldi, I was doing some tests and it seems to me that this change works perfectly, also taking advantage of this improvement, we took the liberty to upload this change to the sandbox (palm version) so you can corroborate that it works perfectly if you want :)

@mariajgrimaldi mariajgrimaldi merged commit 5f9a30e into openedx:open-release/palm.master Apr 4, 2024
81 checks passed
@openedx-webhooks
Copy link

@Anas12091101 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
open-source-contribution PR author is not from Axim or 2U waiting for eng review PR is ready for review. Review and merge it, or suggest changes.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants