Bug 22660 - Allow use of CodeMirror for editing HTML in the news editor
Summary: Allow use of CodeMirror for editing HTML in the news editor
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Lucas Gass
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 26121 26124 26431
  Show dependency treegraph
 
Reported: 2019-04-08 18:14 UTC by Katrin Fischer
Modified: 2020-09-10 19:05 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
20.11.00, 20.05.03


Attachments
Bug 22660: Add system pref to allow switching of editors in News Tool (5.83 KB, patch)
2020-05-08 23:01 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 22660: Add system pref to allow switching of editors in News Tool (5.82 KB, patch)
2020-05-15 15:02 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 22660: Add system pref to allow switching of editors in News Tool (5.85 KB, patch)
2020-05-15 15:08 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 22660: Add system pref to allow switching of editors in News Tool (5.85 KB, patch)
2020-05-19 22:44 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 22660: Add system pref to allow switching of editors in News Tool (5.86 KB, patch)
2020-05-20 16:06 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 22660: Add system pref to allow switching of editors in News Tool (5.91 KB, patch)
2020-07-23 18:15 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 22660: (follow-up) Improve asset handling, add linting (6.00 KB, patch)
2020-07-23 18:15 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 22660: (follow-up) Improve asset handling, add linting (6.05 KB, patch)
2020-07-23 18:20 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 22660: (follow-up) Re-word system preference description (1.28 KB, patch)
2020-07-23 18:23 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 22660: Add system pref to allow switching of editors in News Tool (5.96 KB, patch)
2020-07-24 16:34 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 22660: (follow-up) Improve asset handling, add linting (6.10 KB, patch)
2020-07-24 16:34 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 22660: (follow-up) Re-word system preference description (1.33 KB, patch)
2020-07-24 16:34 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 22660: Add system pref to allow switching of editors in News Tool (6.02 KB, patch)
2020-07-29 22:54 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 22660: (follow-up) Improve asset handling, add linting (6.16 KB, patch)
2020-07-29 22:54 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 22660: (follow-up) Re-word system preference description (1.39 KB, patch)
2020-07-29 22:54 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 22660: (QA follow-up) Fix capitalization in system preference (1.08 KB, patch)
2020-07-29 22:54 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 22660: (QA follow-up) Fix capitalization in system preference (1.07 KB, patch)
2020-07-30 14:03 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2019-04-08 18:14:07 UTC
At the moment when editing news item (and with bug 22318 possible other content) in the OPAC, the WYSIWYG editor is default and you can choose to edit HTML by clicking a button. The button will open the editor in a small pop-up window. There is no syntax highlighting.
To save your changes you have to first click on "Update" and then save the entry.

It would be nice if there was a nicer way involving less clicks working with HTML

Working with HTML allows you to add classes and ids for styling, which will be very helpful for the content areas outside of the news.
Comment 1 Katrin Fischer 2020-03-26 22:53:55 UTC
Lucas proposed a good idea on bug 24764: Use CodeMirror!

Changing the bug title to advertise the idea a bit more.
Comment 2 Lucas Gass 2020-03-26 23:01:57 UTC
With more system preferences like OPACHeader(bz22880) and OPACMainUserBlock (bz23794) moving into the News feature there needs to be a robust way of editing HTML, I think CodeMirror is appropriate.
Comment 3 David Cook 2020-03-26 23:41:21 UTC
Personally, I'd prefer to use CodeMirror. However, my librarians would rather use a WYSIWYG editor I think, so I think being able to do an either/or would be great.
Comment 4 Lucas Gass 2020-05-08 23:01:56 UTC Comment hidden (obsolete)
Comment 5 Sally 2020-05-15 11:12:15 UTC
This fails on step 3 - neither WYSIWYG or CodeMirror loads after applying the patch.  Instead, it's a plain textbox rather than the numbered lines you see with CodeMirror.  If you're editing an existing news item, the plain textbox contains HTML.

When you check NewsToolEditor, it has defaulted to CodeMirror rather than staying as WYSIWYG after the patch has applied.
Comment 6 Lucas Gass 2020-05-15 15:02:30 UTC Comment hidden (obsolete)
Comment 7 Lucas Gass 2020-05-15 15:04:50 UTC
Sally,

Thank you for testing. I made some corrections that should prevent what happened to you in step 3. I also updated the test plan to remind testers to run updatedatabase after applying the patch. 

I have reset the bug status to 'Needs Signoff'.
Comment 8 Lucas Gass 2020-05-15 15:08:07 UTC Comment hidden (obsolete)
Comment 9 Sally 2020-05-15 15:10:58 UTC
(In reply to Lucas Gass from comment #7)
> Sally,
> 
> Thank you for testing. I made some corrections that should prevent what
> happened to you in step 3. I also updated the test plan to remind testers to
> run updatedatabase after applying the patch. 
> 
> I have reset the bug status to 'Needs Signoff'.

Great, thanks Lucas - I'll give it another go.
Comment 10 Sally 2020-05-15 15:38:56 UTC
I think I'm going to have to admit defeat - I can't see how to update the database using a sandbox, and from your comments, I think that's the cause of the errors I'm seeing.

I'll leave it for someone else to test - it would be a useful feature.  :)
Comment 11 Lucas Gass 2020-05-19 22:44:36 UTC Comment hidden (obsolete)
Comment 12 Lucas Gass 2020-05-20 16:06:30 UTC Comment hidden (obsolete)
Comment 13 Owen Leonard 2020-07-23 18:15:27 UTC
Created attachment 107267 [details] [review]
Bug 22660: Add system pref to allow switching of editors in News Tool

This patch adds a system preference called 'NewsToolEditor' which allows for the choice between TinyMCE and CodeMirror in the News Feature.

TEST PLAN:
1. Apply patch and run updatedatabase
2. Go to Tools > News and make a new entry or edit an existing News item.
3. When you initially load you should gte the TinyMCE editor (WYSIWYG)
4. Search for the NewsToolEditor system preference and switch to CodeMirror.
5. Reload your News item and now you should see the CodeMirror editor.
6. Try saving content with both and maki sure it works.
7. Try adding some different HTML tags in CodeMirror mode, saving, and making sure it looks right when you switch back to TinyMCE

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 14 Owen Leonard 2020-07-23 18:15:31 UTC Comment hidden (obsolete)
Comment 15 Owen Leonard 2020-07-23 18:20:23 UTC
Created attachment 107269 [details] [review]
Bug 22660: (follow-up) Improve asset handling, add linting

This patch makes some changes to the way assets are included based on
the value of the system preferece in order to minimize unnecessary
JavaScript includes.

The patch also adds HTML syntax-highlighting and linting like we have in
the system preferences editor.

Removed are two JS files which were not required.

To test, apply the patch and set the NewsToolEditor preference to
"CodeMirror."

 - Edit a news item and confirm that the contents of the
   editor show HTML syntax highlighting.
 - Test HTML linting by adding some malformed HTML (missing closing tag,
   for instance). The error should be highlighted.
 - Set the NewsToolEditor to "TinyMCE" and confirm that WYSIWYG editing
   still works correctly.

https://bugs.koha-community.org/show_bug.cgi?id=22600
Comment 16 Owen Leonard 2020-07-23 18:23:33 UTC
Created attachment 107270 [details] [review]
Bug 22660: (follow-up) Re-word system preference description

This patch rewords the system preference description to focus on the
generic description of the editor rather than the name of the plugin:

    tinymce: "a WYSIWYG editor (TinyMCE)"
    codemirror: "a text editor (CodeMirror)"
Comment 17 ByWater Sandboxes 2020-07-24 16:34:39 UTC
Created attachment 107361 [details] [review]
Bug 22660: Add system pref to allow switching of editors in News Tool

This patch adds a system preference called 'NewsToolEditor' which allows for the choice between TinyMCE and CodeMirror in the News Feature.

TEST PLAN:
1. Apply patch and run updatedatabase
2. Go to Tools > News and make a new entry or edit an existing News item.
3. When you initially load you should gte the TinyMCE editor (WYSIWYG)
4. Search for the NewsToolEditor system preference and switch to CodeMirror.
5. Reload your News item and now you should see the CodeMirror editor.
6. Try saving content with both and maki sure it works.
7. Try adding some different HTML tags in CodeMirror mode, saving, and making sure it looks right when you switch back to TinyMCE

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Lisette Scheer <lisetteslatah@gmail.com>
Comment 18 ByWater Sandboxes 2020-07-24 16:34:41 UTC
Created attachment 107362 [details] [review]
Bug 22660: (follow-up) Improve asset handling, add linting

This patch makes some changes to the way assets are included based on
the value of the system preferece in order to minimize unnecessary
JavaScript includes.

The patch also adds HTML syntax-highlighting and linting like we have in
the system preferences editor.

Removed are two JS files which were not required.

To test, apply the patch and set the NewsToolEditor preference to
"CodeMirror."

 - Edit a news item and confirm that the contents of the
   editor show HTML syntax highlighting.
 - Test HTML linting by adding some malformed HTML (missing closing tag,
   for instance). The error should be highlighted.
 - Set the NewsToolEditor to "TinyMCE" and confirm that WYSIWYG editing
   still works correctly.

https://bugs.koha-community.org/show_bug.cgi?id=22600

Signed-off-by: Lisette Scheer <lisetteslatah@gmail.com>
Comment 19 ByWater Sandboxes 2020-07-24 16:34:44 UTC
Created attachment 107363 [details] [review]
Bug 22660: (follow-up) Re-word system preference description

This patch rewords the system preference description to focus on the
generic description of the editor rather than the name of the plugin:

    tinymce: "a WYSIWYG editor (TinyMCE)"
    codemirror: "a text editor (CodeMirror)"

Signed-off-by: Lisette Scheer <lisetteslatah@gmail.com>
Comment 20 Lisette Scheer 2020-07-24 16:35:46 UTC
Worked great! I switched back and forth a few times making different changes each time and everything worked well with both editors.
Comment 21 Katrin Fischer 2020-07-29 22:54:12 UTC
Created attachment 107554 [details] [review]
Bug 22660: Add system pref to allow switching of editors in News Tool

This patch adds a system preference called 'NewsToolEditor' which allows for the choice between TinyMCE and CodeMirror in the News Feature.

TEST PLAN:
1. Apply patch and run updatedatabase
2. Go to Tools > News and make a new entry or edit an existing News item.
3. When you initially load you should gte the TinyMCE editor (WYSIWYG)
4. Search for the NewsToolEditor system preference and switch to CodeMirror.
5. Reload your News item and now you should see the CodeMirror editor.
6. Try saving content with both and maki sure it works.
7. Try adding some different HTML tags in CodeMirror mode, saving, and making sure it looks right when you switch back to TinyMCE

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Lisette Scheer <lisetteslatah@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 22 Katrin Fischer 2020-07-29 22:54:16 UTC
Created attachment 107555 [details] [review]
Bug 22660: (follow-up) Improve asset handling, add linting

This patch makes some changes to the way assets are included based on
the value of the system preferece in order to minimize unnecessary
JavaScript includes.

The patch also adds HTML syntax-highlighting and linting like we have in
the system preferences editor.

Removed are two JS files which were not required.

To test, apply the patch and set the NewsToolEditor preference to
"CodeMirror."

 - Edit a news item and confirm that the contents of the
   editor show HTML syntax highlighting.
 - Test HTML linting by adding some malformed HTML (missing closing tag,
   for instance). The error should be highlighted.
 - Set the NewsToolEditor to "TinyMCE" and confirm that WYSIWYG editing
   still works correctly.

https://bugs.koha-community.org/show_bug.cgi?id=22600

Signed-off-by: Lisette Scheer <lisetteslatah@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 23 Katrin Fischer 2020-07-29 22:54:21 UTC
Created attachment 107556 [details] [review]
Bug 22660: (follow-up) Re-word system preference description

This patch rewords the system preference description to focus on the
generic description of the editor rather than the name of the plugin:

    tinymce: "a WYSIWYG editor (TinyMCE)"
    codemirror: "a text editor (CodeMirror)"

Signed-off-by: Lisette Scheer <lisetteslatah@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 24 Katrin Fischer 2020-07-29 22:54:26 UTC
Created attachment 107557 [details] [review]
Bug 22660: (QA follow-up) Fix capitalization in system preference

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 25 Katrin Fischer 2020-07-29 22:55:34 UTC
I think the CodeMirror option is much needed as we move more OPAC prefs, so really happy to see these patches.

I think a nice improvement could be to have a "both" option - making both available and allow to choose, depending on what you are editing (former pref or news entry)

While I and my coworkers will probably prefer HTML for editing the OPAC related bits, the libraries we support will more likely prefer the WYSIWYG for adding/editing news. 
 
Maybe by having 2 different Edit buttons/options or a split button, defaulting to  the editor from the pref?
Comment 26 Jonathan Druart 2020-07-30 14:03:48 UTC
Created attachment 107592 [details] [review]
Bug 22660: (QA follow-up) Fix capitalization in system preference

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 27 Jonathan Druart 2020-07-30 14:04:20 UTC
(fixing patch's author)
Comment 28 Jonathan Druart 2020-07-30 15:46:32 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 29 Katrin Fischer 2020-07-30 19:45:34 UTC
Oops, thx Jonathan :)
Comment 30 Lucas Gass 2020-08-06 20:34:27 UTC
If you work with the HTML system prefs a lot, which are moving into the News tool, this enhancement could be pretty important. Choosing to backport to 20.05.x for 20.05.03
Comment 31 Aleisha Amohia 2020-08-07 02:14:15 UTC
enhancement changing too much behaviour, not backported to 19.11.x