Skip to content

Conversation

@Fedik
Copy link
Member

@Fedik Fedik commented Jul 31, 2023

Pull Request for Issue #41278 .

Summary of Changes

(Re)Writing a plugin for TinyMCE code highlighting with CodeMirror 6.

Testing Instructions

Apply patch, run npm install.
Enable "code highlighting" in TinyMCE plugin parameters.
Open article editing and check that "code highlighting" is working

Actual result BEFORE applying this Pull Request

working

Expected result AFTER applying this Pull Request

working

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:
  • No documentation changes for docs.joomla.org needed
  • Pull Request link for manual.joomla.org:
  • No documentation changes for manual.joomla.org needed

@Fedik Fedik marked this pull request as draft July 31, 2023 14:23
@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-5.0-dev labels Jul 31, 2023
@dgrammatiko
Copy link
Contributor

@Fedik wasn't the reason that the original version used an iframe to not interfere with the tinyMCE styles?

Anyways, you could wrap the textarea into a web component with a slot (with Shadow DOM) to overcome the styling problem...

@Fedik
Copy link
Member Author

Fedik commented Jul 31, 2023

wasn't the reason that the original version used an iframe to not interfere with the tinyMCE styles?

I think it was more about avoid of loading codemirror scripts and styles, on initila load. But maybe also this.

web component with a slot (with Shadow DOM) to overcome the styling problem

That what I want to avoid: shadow dom and iframe :)
It has own limitation. In any case will see, in the end can be on of that.
Also we can use own dialog #40150

@Fedik Fedik marked this pull request as ready for review August 4, 2023 15:11
@Fedik
Copy link
Member Author

Fedik commented Aug 4, 2023

This is ready for testing

@HLeithner
Copy link
Member

@brianteeman you may have time tomorrow for a quick look?

@HLeithner
Copy link
Member

it works for me but looks different then 4.4 version right?

@HLeithner
Copy link
Member

4.4
image
this pr:
image

shortcuts are working but a bit hard to know if they are not listed anywhere

@brianteeman
Copy link
Contributor

  1. Agree with @HLeithner about it missing the legends for search
  2. Needs code to remove the old plugin
  3. The tab key is not usable in the editor (it was in j43)
  4. Using the tab key the focus is not trapped in the modal. As a result you can tab away from the modal and it will stay open and youn cannot close it (a11y)

@Fedik
Copy link
Member Author

Fedik commented Aug 10, 2023

@HLeithner you forgot to enable "Source Code Highlighting" in plugin parameters.

missing the legends for search

It is gone, forever, sorry.

The tab key is not usable in the editor (it was in j43)

It is general behavior of codemirror 6, check codemirror/dev#238 Quote:

This is by design, since tab is an essential part of keyboard web navigation, and trapping it harms accessibility.

It can be coded to work, but do we need it?

Using the tab key the focus is not trapped in the modal

Hmhm, this is working with default "Source view" of TinyMCE.
Probably something with codemirror 6 and tab handling in general.
Need to look.

@HLeithner
Copy link
Member

I'm merging this for now, but the tab issues is a bit annoying, I have read suggestions like key combination ESC and then TAB to escape from the editor area. Which sounds good to me.

@brianteeman
Copy link
Contributor

The accessibility issue really must be fixed before merge

@Fedik
Copy link
Member Author

Fedik commented Aug 11, 2023

I'm merging this for now

Do not hurry, it is not that important feature, and it is optional anyway.
I will look in to tab thing some time, this or next week.

The tab escaping the modal is easy to fix with dialog modal (#40150), the browser automatically prevents the focus to leave the dialog.
But I think it also can be fixed when enable Codemirror to trap the tab, as it done in 4.x

@dgrammatiko
Copy link
Contributor

@Fedik just switch to the Joomla modal and ask @HLeithner to finally merge that PR...

@Fedik
Copy link
Member Author

Fedik commented Aug 13, 2023

I will never get into the editor again

At this point TinyMCE trap the tab.

@HLeithner
Copy link
Member

I will never get into the editor again

At this point TinyMCE trap the tab.

I'm still in the modal and can tab between 3 modal buttons but can't get back into CM

@Fedik
Copy link
Member Author

Fedik commented Aug 13, 2023

I'm still in the modal and can tab between 3 modal buttons but can't get back into CM

That what I meant, TinyMCE "tab trap" does not allow for the focus to go in to content, in some reason.

I replaced one bug with annother.
New one probably better one 😄

@HLeithner
Copy link
Member

but at this point in time tinymce isn't in charge of doing anything or?

@Fedik
Copy link
Member Author

Fedik commented Aug 13, 2023

but at this point in time tinymce isn't in charge of doing anything or?

It is, the modal is from tinymce API, it have a feature to "trap the tab", to avoind a bug when tab goes under the modal.
when you push shift+tab, the code changes the focus from codemirror to save/cancel button of tinymce, and it is stuck there,

@HLeithner
Copy link
Member

ok, now I understand, can this be solved with your own modal #40150 ?

@Fedik
Copy link
Member Author

Fedik commented Aug 13, 2023

yeap, I tried a quick test, it is working.
Can do like this, if you like: you can merge current PR, at least it is usable.
I plan a litle changes for #40150, then it also can be merged.
After that I can update the codemirror + tinymce to use our dialog instead of tinymce.

@HLeithner HLeithner merged commit 6ba6cbd into joomla:5.0-dev Aug 14, 2023
@HLeithner
Copy link
Member

Ok then I merge this for now, even if it's not optimal and hope that with #40150 we get a proper working interface.

thanks

@HLeithner HLeithner added this to the Joomla! 5.0 milestone Aug 14, 2023
@Fedik Fedik deleted the tinymce-highlght6 branch August 14, 2023 08:09
GeraintEdwards pushed a commit to GeraintEdwards/joomla-cms that referenced this pull request Aug 14, 2023
* joomla-highlighter

* joomla-highlighter

* fixes

* joomla-highlighter

* clean

* clean

* focus

* Fix styling

* Tab trap
richard67 added a commit to richard67/joomla-cms that referenced this pull request Aug 19, 2023
richard67 added a commit to richard67/joomla-cms that referenced this pull request Aug 19, 2023
richard67 added a commit to richard67/joomla-cms that referenced this pull request Aug 19, 2023
HLeithner pushed a commit that referenced this pull request Aug 20, 2023
* Add deleted files and folders and renamed files

- Deleted files and folders from PR #41276
- Renamed file from PR #41260

* Migrate enabled and params from old system plugin on update

* Remove deleted files and folders from PR #41276

These files and folders will be removed by the uninstallation of the plugin.

* Add exceptions to deleted file check script

These files and folders will be removed by the uninstallation of the plugin and therefore should not be added to the lists of deleted files and folders.

* Add deleted files and folders from PR #41289

* Add /media/plg_system_compat to exceptions

* Remove /media/plg_system_compat deleted files and folders

* Revert "Remove /media/plg_system_compat deleted files and folders"

This reverts commit ea429a6.

* Revert "Add /media/plg_system_compat to exceptions"

This reverts commit 296febf.
richard67 added a commit to richard67/joomla-cms that referenced this pull request Sep 1, 2023
richard67 added a commit to richard67/joomla-cms that referenced this pull request Sep 4, 2023
richard67 added a commit to richard67/joomla-cms that referenced this pull request Sep 4, 2023
richard67 added a commit to richard67/joomla-cms that referenced this pull request Sep 4, 2023
richard67 added a commit to richard67/joomla-cms that referenced this pull request Sep 5, 2023
richard67 added a commit to richard67/joomla-cms that referenced this pull request Sep 14, 2023
richard67 added a commit to richard67/joomla-cms that referenced this pull request Sep 19, 2023
richard67 added a commit to richard67/joomla-cms that referenced this pull request Sep 26, 2023
richard67 added a commit to richard67/joomla-cms that referenced this pull request Sep 26, 2023
richard67 added a commit to richard67/joomla-cms that referenced this pull request Sep 26, 2023
richard67 added a commit to richard67/joomla-cms that referenced this pull request Sep 26, 2023
richard67 added a commit to richard67/joomla-cms that referenced this pull request Oct 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Feature NPM Resource Changed This Pull Request can't be tested by Patchtester

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants