Skip to content

Removing Froala JS #14501

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Feb 24, 2025
Merged

Removing Froala JS #14501

merged 4 commits into from
Feb 24, 2025

Conversation

escopecz
Copy link
Member

@escopecz escopecz commented Jan 26, 2025 •
edited
Loading

Q A
Bug fix? (use the a.b branch) 🟢
New feature/enhancement? (use the a.x branch) 🔴
Deprecations? 🔴
BC breaks? (use the c.x branch) 🟢
Automated tests included? 🔴 (not possible for removed code)
Related user documentation PR URL mautic/user-documentation#...
Related developer documentation PR URL mautic/developer-documentation-new#...
Issue(s) addressed Fixes #...

Description

This PR depends on #14450. Please review and test this one once that one is merged and this one rebased.

Finally removing the Froala JS text editor that is outdated, with security vulnerabilities and now unused when the legacy builder was removed.

📋 Steps to test this PR:

  1. Open this PR on Gitpod or pull down for testing locally (see docs on testing PRs here)
  2. Test that editing text in Mautic works wherever you can think of. Mostly builders and description editors.

@escopecz escopecz added the blocked Something blocks this PR/issue (e.g. waiting for another PR to be merged) label Jan 26, 2025
@escopecz escopecz added this to the 6.0 milestone Jan 26, 2025
@escopecz escopecz added the bc-break A BC break PR for major release milestones only label Jan 26, 2025
@RCheesley RCheesley modified the milestones: 6.0.0-alpha, 6.0.0-beta Jan 27, 2025
@matbcvo matbcvo removed the blocked Something blocks this PR/issue (e.g. waiting for another PR to be merged) label Feb 24, 2025
@codecov Codecov
Copy link

codecov bot commented Feb 24, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.25%. Comparing base (02080c2) to head (d845032).
Report is 4 commits behind head on 6.x.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##                6.x   #14501   +/-   ##
=========================================
  Coverage     64.24%   64.25%           
+ Complexity    34576    34569    -7     
=========================================
  Files          2268     2268           
  Lines        103294   103258   -36     
=========================================
- Hits          66366    66353   -13     
+ Misses        36928    36905   -23
Files with missing lines Coverage Δ
...p/bundles/CoreBundle/Controller/FileController.php 51.72% <ø> (ø)
...undles/CoreBundle/EventListener/CoreSubscriber.php 60.17% <ø> (-2.01%) ⬇️
app/bundles/CoreBundle/Form/Type/ConfigType.php 100.00% <ø> (ø)
...pp/bundles/CoreBundle/Twig/Helper/AssetsHelper.php 77.64% <ø> (+6.58%) ⬆️

... and 1 file with indirect coverage changes

@escopecz escopecz marked this pull request as ready for review February 24, 2025 10:46
@escopecz escopecz added ready-to-test PR's that are ready to test code-review-needed PR's that require a code review before merging labels Feb 24, 2025
matbcvo
matbcvo approved these changes Feb 24, 2025
View reviewed changes
Copy link
Contributor

@matbcvo matbcvo left a comment

Choose a reason for hiding this comment

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

The code changes look good to me. I’ve tested the PR and found no issues. 👍

@matbcvo matbcvo added pending-test-confirmation PR's that require one test before they can be merged code-review-passed PRs which have passed code review and removed ready-to-test PR's that are ready to test code-review-needed PR's that require a code review before merging labels Feb 24, 2025
imaabasiee
imaabasiee approved these changes Feb 24, 2025
View reviewed changes
Copy link

@imaabasiee imaabasiee left a comment

Choose a reason for hiding this comment

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

Tested this pr and found no issue editing text anywhere on mautic.

@matbcvo matbcvo added ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged user-testing-passed PRs which have been successfully tested by the required number of people. and removed pending-test-confirmation PR's that require one test before they can be merged labels Feb 24, 2025
@RCheesley RCheesley merged commit 6dfb228 into mautic:6.x Feb 24, 2025
17 checks passed
@RCheesley RCheesley modified the milestones: 6.0.0-beta, 6.0.0-beta2 Mar 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matbcvo matbcvo matbcvo approved these changes

+1 more reviewer

@imaabasiee imaabasiee imaabasiee approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
bc-break A BC break PR for major release milestones only code-review-passed PRs which have passed code review ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged user-testing-passed PRs which have been successfully tested by the required number of people.
Projects
Open Source Fridays
Status: 🥳 Done
Milestone
6.0.0-beta2
Development

Successfully merging this pull request may close these issues.
4 participants
@escopecz @matbcvo @imaabasiee @RCheesley