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

Ckedito5 patch for table cell resize #533

Merged
merged 11 commits into from
Nov 25, 2024

Conversation

sharmasahil
Copy link
Contributor

@sharmasahil sharmasahil commented Oct 31, 2024

Jira

SD-358 - Control WYSIWYG and data table column widths in ckeditor

Fix

  1. Removed local patch as it got issues with retaining the styles while we edit the node again, we are already using Limit allowed html filter which is causing it.
  2. Found this sandbox module https://git.drupalcode.org/sandbox/s_leu-3413350/-/tree/1.0.x which solves this issue and allows table inline styles to be saved correctly. It provides a new filter in text format 'Resize table columns' enabling it sorts col resize and retain styles on edit as well.
  3. Updated basic enhancer to provide table and col inline styles to FE.

Copy link
Contributor

@vincent-gao vincent-gao left a comment

Choose a reason for hiding this comment

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

hey @sharmasahil
do you need an update hook to apply the changes across all sites?
Also, could you clarify the need for a local patch?

@sharmasahil
Copy link
Contributor Author

hey @sharmasahil do you need an update hook to apply the changes across all sites? Also, could you clarify the need for a local patch?

Thanks for the review @vincent-gao, I have updated the PR with comments and also removed local patch, put install hook too.

Copy link
Contributor

@vincent-gao vincent-gao left a comment

Choose a reason for hiding this comment

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

hey @sharmasahil
Just a few minor things, and it's perfectly fine if you don't want to make any changes.
please make sure build and tests pass.


// Extract only the inner HTML of the <body> tag, avoiding extra tags.
$processedBodyContent = '';
foreach ($processedDom->getElementsByTagName('body')->item(0)->childNodes as $node) {
Copy link
Contributor

Choose a reason for hiding this comment

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

can this be simplified by using DOMDocument::saveHTML directly on the element.
e.g.

$processedBodyContent = $processedDom->saveHTML($processedDom->getElementsByTagName('body')->item(0));

libxml_use_internal_errors(true);
$valueDom->loadHTML($data['value']);
$processedDom->loadHTML($data['processed']);
libxml_clear_errors();
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@vincent-gao vincent-gao merged commit d0a79d4 into develop Nov 25, 2024
1 check failed
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.

2 participants