Skip to content

Comments

MIR-1417 replace ckeditor with tinymce#1100

Merged
Mewel merged 2 commits into2023.06.xfrom
issues/MIR-1417-replace-ckeditor-with-tinymce
Mar 31, 2025
Merged

MIR-1417 replace ckeditor with tinymce#1100
Mewel merged 2 commits into2023.06.xfrom
issues/MIR-1417-replace-ckeditor-with-tinymce

Conversation

@Mewel
Copy link
Member

@Mewel Mewel commented Dec 19, 2024

@Mewel Mewel requested review from sebhofmann and yagee-de December 19, 2024 13:34
@yagee-de
Copy link
Member

please port back to 2023.06.x

@Mewel Mewel force-pushed the issues/MIR-1417-replace-ckeditor-with-tinymce branch from 8d78cbc to b1fef03 Compare January 15, 2025 13:02
@Mewel Mewel changed the base branch from main to 2023.06.x January 15, 2025 13:03
Copy link
Member

@sebhofmann sebhofmann left a comment

Choose a reason for hiding this comment

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

Looks good. But the Tests are broken:

Expected condition failed: waiting for presence of any elements located by By.xpath: .//div[contains(@id,'mods:abstract[1]') and contains(@class, 'cke')]//iframe (tried for 30 

@toKrause
Copy link
Contributor

toKrause commented Feb 21, 2025

Any idea how to effectively replace MIR.WebConfig.Editor.CKEditor.Wordcount.MaxCharCount in a way that can still be coordinated with XEditor validaten rules?


Example with CKEditor and a character count limit of 2000:

MIR.WebConfig.Editor.CKEditor.Wordcount.CountBytesAsChars=true
MIR.WebConfig.Editor.CKEditor.Wordcount.CountSpacesAsChars=true
MIR.WebConfig.Editor.CKEditor.Wordcount.CountHTML=true
MIR.WebConfig.Editor.CKEditor.Wordcount.CountLineBreaks=true
MIR.WebConfig.Editor.CKEditor.Wordcount.MaxCharCount=2000
<mir:htmlArea xpath="." label="mir.abstract.ortext" help-text="{i18n:mir.help.abstract.ortext}" rows="5" />
<xed:validate display="global" matches="^[\S\s]{1,2001}$" i18n="metadata.validation.abstract.maxLength"/>

@toKrause toKrause changed the title replace ckeditor with tinymce MIR-1417 replace ckeditor with tinymce Feb 25, 2025
@Mewel
Copy link
Member Author

Mewel commented Mar 26, 2025

Any idea how to effectively replace MIR.WebConfig.Editor.CKEditor.Wordcount.MaxCharCount in a way that can still be coordinated with XEditor validaten rules?

Example with CKEditor and a character count limit of 2000:

MIR.WebConfig.Editor.CKEditor.Wordcount.CountBytesAsChars=true
MIR.WebConfig.Editor.CKEditor.Wordcount.CountSpacesAsChars=true
MIR.WebConfig.Editor.CKEditor.Wordcount.CountHTML=true
MIR.WebConfig.Editor.CKEditor.Wordcount.CountLineBreaks=true
MIR.WebConfig.Editor.CKEditor.Wordcount.MaxCharCount=2000
<mir:htmlArea xpath="." label="mir.abstract.ortext" help-text="{i18n:mir.help.abstract.ortext}" rows="5" />
<xed:validate display="global" matches="^[\S\s]{1,2001}$" i18n="metadata.validation.abstract.maxLength"/>

I'm not really familiar on how the xeditor validates this. But there is a plugin for tinymce: https://www.tiny.cloud/docs/tinymce/latest/wordcount/ which is also activated by default. So maybe you can use that?

@fluetze
Copy link
Member

fluetze commented Mar 26, 2025

The only thing xeditor validates against is the given pattern, and it is done server-side after form submission.
<xed:validate display="global" matches="^[\S\s]{1,2001}$" i18n="metadata.validation.abstract.maxLength"/>
The discussion here is about client-side validation.

See xeditor documentation: A possible solution to force validation to be same on client and server side is to use a custom java class that implements it in the server. That class could read the properties and test the input.

@toKrause
Copy link
Contributor

Yes, the idea is to have the same validation on client side and on server side, such that users have a visual feedback while entering (or after pasting) some text; but the actual validation needs to be done on the server side. Users should be able to temporarily enter (or paste) a text, that exceeds the limits.

The configuration given above achieves this effectively with the current editor.

I've already looked at the plugin you mentioned, but it appears to be simply informative. It has no concept of a limit. Displaying the character count might be possible (https://www.tiny.cloud/blog/set-character-count-limit/#h_72792831019251667464431803). A custom plugin might be able to change the text color of the character count to red, if a given limit is exceeded.

Creating a custom validator implementation in Java might actually be an easier approach than trying to force a enforce a limit otherwise. Good idea.

@Mewel Mewel force-pushed the issues/MIR-1417-replace-ckeditor-with-tinymce branch from 0a610f5 to fe89d63 Compare March 27, 2025 14:12
@Mewel Mewel removed the request for review from yagee-de March 27, 2025 14:45
@Mewel
Copy link
Member Author

Mewel commented Mar 31, 2025

Can we merge this PR?

@toKrause
Copy link
Contributor

Can we merge this PR?

I'll have to look into how I can coordinate client-side and server-side validation, but that shouldn't impede merging this PR.

Since this is a drastic change, it should be done in main, but I don't think that that is a popular opinion in this case.

@Mewel
Copy link
Member Author

Mewel commented Mar 31, 2025

I was asked to do this for 2023, so I guess not.

@yagee-de on Jan 14
please port back to 2023.06.x

@Mewel Mewel merged commit 4564202 into 2023.06.x Mar 31, 2025
2 checks passed
@Mewel Mewel deleted the issues/MIR-1417-replace-ckeditor-with-tinymce branch March 31, 2025 08:47
yagee-de added a commit that referenced this pull request Apr 10, 2025
* 2023.06.x:
  MIR-1362 fix bug for secondary search form if Solr RequestHandler is not /find (#1133)
  MIR-1437 Display language next to tab title (#1139)
  MIR-1441 Display proper message to user when history is currently not avilable (#1138)
  MIR-1417 replace ckeditor with tinymce (#1100)
  MIR-1429 enhance podcast RSS feed (#1112)
  MIR-1429 bugfix: do not require thumbnail for item
  remove old code after conflict resolution and merge
  MIR-1362 revert to test of the dropdown element
  MIR-1362 revert to modified bootstrap dropdown element instead of select box in secondary search
  MIR-1362 add wrapper with css classes to the select box
  MIR-1362 fix initialCondQuery hidden input field value
  MIR-1362 change for the hidden parameter initialCondQuery
  MIR-1362 clean up unused code
  fix form for the case with solr/select
  MIR-1362 fix the query with the servlets/solr/select path
  MIR-1362 fix empty request in the the main search
  MIR-1362 fix empty fq and queries with more than one word
  MIR-1362 fix empty value in the input query field, fix unit test
  MIR-1362 change and fix the second search with filter query etc
  MIR-1362 fix SOLR-request for all fields
  MIR-1362 fix SOLR query filter
yagee-de added a commit that referenced this pull request Apr 10, 2025
* 2024.06.x:
  MIR-1362 fix bug for secondary search form if Solr RequestHandler is not /find (#1133)
  MIR-1437 Display language next to tab title (#1139)
  MIR-1441 Display proper message to user when history is currently not avilable (#1138)
  MIR-1417 replace ckeditor with tinymce (#1100)
  MIR-1429 enhance podcast RSS feed (#1112)
  MIR-1429 bugfix: do not require thumbnail for item
  remove old code after conflict resolution and merge
  MIR-1362 revert to test of the dropdown element
  MIR-1362 revert to modified bootstrap dropdown element instead of select box in secondary search
  MIR-1362 add wrapper with css classes to the select box
  MIR-1362 fix initialCondQuery hidden input field value
  MIR-1362 change for the hidden parameter initialCondQuery
  MIR-1362 clean up unused code
  fix form for the case with solr/select
  MIR-1362 fix the query with the servlets/solr/select path
  MIR-1362 fix empty request in the the main search
  MIR-1362 fix empty fq and queries with more than one word
  MIR-1362 fix empty value in the input query field, fix unit test
  MIR-1362 change and fix the second search with filter query etc
  MIR-1362 fix SOLR-request for all fields
  MIR-1362 fix SOLR query filter
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.

5 participants