Feature Language Change Documentation Required NPM Resource Changed PR-5.0-dev b/c break Pending

User tests: Successful: Unsuccessful:

avatar Fedik
Fedik
27 Jun 2023

Summary of Changes

Update Codemirror 6

I removed half of old parameters from plugin configuration.
But I think no one will notice.

Testing Instructions

Apply patch, run npm install
Change default editor to codemirror, make sure editing still works

Actual result BEFORE applying this Pull Request

Works

Expected result AFTER applying this Pull Request

Works

Link to documentations

Please select:

Links

avatar joomla-cms-bot joomla-cms-bot - change - 27 Jun 2023
Category Administration Language & Strings JavaScript Repository NPM Change Front End Plugins
avatar Fedik Fedik - open - 27 Jun 2023
avatar Fedik Fedik - change - 27 Jun 2023
Status New Pending
avatar Fedik Fedik - change - 27 Jun 2023
Labels Added: Feature Language Change Documentation Required NPM Resource Changed PR-5.0-dev b/c break
avatar brianteeman
brianteeman - comment - 27 Jun 2023

npm i had the following in its output



Building Media Manager ES Module...
'@codemirror/view' is imported by build/media_source/plg_editors_codemirror/js/codemirror-base.es6.js, but could not be resolved – treating it as an external dependency
'@codemirror/state' is imported by build/media_source/plg_editors_codemirror/js/codemirror-base.es6.js, but could not be resolved – treating it as an external dependency
'@codemirror/language' is imported by build/media_source/plg_editors_codemirror/js/codemirror-base.es6.js, but could not be resolved – treating it as an external dependency
'@codemirror/commands' is imported by build/media_source/plg_editors_codemirror/js/codemirror-base.es6.js, but could not be resolved – treating it as an external dependency
'@codemirror/search' is imported by build/media_source/plg_editors_codemirror/js/codemirror-base.es6.js, but could not be resolved – treating it as an external dependency
'@codemirror/autocomplete' is imported by build/media_source/plg_editors_codemirror/js/codemirror-base.es6.js, but could not be resolved – treating it as an external dependency
'codemirror' is imported by build/media_source/plg_editors_codemirror/js/joomla-editor-codemirror.w-c.es6.js, but could not be resolved – treating it as an external dependency
'@codemirror/state' is imported by build/media_source/plg_editors_codemirror/js/joomla-editor-codemirror.w-c.es6.js, but could not be resolved – treating it as an external dependency
'@codemirror/view' is imported by build/media_source/plg_editors_codemirror/js/joomla-editor-codemirror.w-c.es6.js, but could not be resolved – treating it as an external dependency
✅ ES2017 file: codemirror-base.js: transpiled

Opening codemirror in the template manager fails with
0 Class "Joomla\Plugin\EditorsXtd\PageBreak\Extension\PageBreak" not found

avatar Fedik
Fedik - comment - 27 Jun 2023

You can ignore these warnings.

0 Class "Joomla\Plugin\EditorsXtd\PageBreak\Extension\PageBreak" not found

Remove autoload_psr4.php, it not related to this PR, but remove it every time you update git repo ;)

avatar richard67
richard67 - comment - 27 Jun 2023

@Fedik Do we need a migration of old plugin parameters on update in script.php like we have added it for TinyMCE with the PR here #40626 ?

avatar Fedik
Fedik - comment - 27 Jun 2023

Nope. There only a few left from old parameters. And they should work.

avatar brianteeman
brianteeman - comment - 27 Jun 2023

silly me

avatar brianteeman
brianteeman - comment - 27 Jun 2023

Code collapse not working correctly. Instead of collapsing the matchig tag it is collapsing everything from the start.

Line Numbering and collapse icon are too close together. Much better before.
image
image

Highlight matching tag on click no longer works. This was very very useful.
image
image

avatar brianteeman
brianteeman - comment - 27 Jun 2023

Toolbar missing when in fullscreen
image
image

avatar Fedik
Fedik - comment - 27 Jun 2023

Code collapse not working correctly
Line Numbering and collapse icon are too close together.

Yes, this is how new version looks like, there nothing from me.

Highlight matching tag on click no longer works. This was very very useful.

It works but a bit different than how it was in older version.
And I noticed it does not works for mixed content php + html.

Toolbar missing when in fullscreen

I will look in to it.

UPD. Btw there an option "Highlight Selection Matches" that will highlight everything, try to enable it, maybe that what you looking for.

avatar richard67
richard67 - comment - 27 Jun 2023

Just as a reference, e.g. for other readers: https://codemirror.net/docs/migration/

@Fedik Maybe you could link that in the description (initial post) of this PR, similar like it was with the TinyMCE PR, so reviewers can check it?

avatar Fedik Fedik - change - 27 Jun 2023
The description was changed
avatar Fedik Fedik - edited - 27 Jun 2023
avatar Fedik
Fedik - comment - 27 Jun 2023

Added

avatar Fedik
Fedik - comment - 28 Jun 2023

Fullscreen should work better now.
Also "Highlight Selection Matches" is On by default

avatar Fedik
Fedik - comment - 28 Jun 2023

Instead of collapsing the matchig tag it is collapsing everything from the start.

It is something to do with <jdoc:include />, codemirror does not like self closed tags in HTML.

Yeah, well, it probably will stay like that codemirror/dev#905
So, just do not try to fold when code contains self closed custom tags :)

avatar dgrammatiko
dgrammatiko - comment - 29 Jun 2023

@Fedik I have one objection on the build tools. Instead of reading the entry file and try to figure out if the module should be bundled or not from a comment you should use a custom specifier (ie instead of import x from 'moduleX' use import x from 'joomla:moduleX') and resolve this WITH a rollup plugin, ie: https://github.com/GoogleChromeLabs/proxx/blob/main/lib/asset-plugin.js

The custom specifier is a technic widely used, actually you can use import fs from 'node:fs' or import fs from 'deno:fs' or import fs from 'bun:fs' depending on the runtime (node, demo, bun).

avatar Fedik
Fedik - comment - 29 Jun 2023

That what I looking for, thanks.
I will update.

Instead of reading the entry file

It only reads first couple lines of the file ;)

avatar dgrammatiko
dgrammatiko - comment - 29 Jun 2023

It only reads first couple lines of the file ;)

My point wasn't any performance issues rather that the approach is extremely narrow (which is ok-ish in the context of the code mirror script). What if, down the road, you want to have some import that would actually needs to be bundled and the other/existing modules need to be external/unresolved? What if later on you need to use a dynamic import?, etc...
I think the custom specifier (I would use external: for this context) is a better solution and could be applied already to the bootstrap scripts as well.

avatar Fedik
Fedik - comment - 29 Jun 2023

you want to have some import that would actually needs to be bundled and the other/existing modules need to be external/unresolved?

Well, I just need rollump to ignore whole file :)
Since it already okaish.

TBH, I hate all this compilations back and forth :neckbeard:
It complicates everything.

I will look, use of external: prefix sounds as not a bad idea.

avatar Fedik
Fedik - comment - 30 Jun 2023

I will look, use of external: prefix sounds as not a bad idea.

Well, that not very good, it is helpes for rollup config for sure, but makes coding crappy: no code navigation, no autocmpletion etc.
I made a regexp for now. Maybe letter will find a better way, or will back to what I made originaly.

I just collecting all externals (once) before build.

avatar Fedik Fedik - change - 1 Jul 2023
The description was changed
avatar Fedik Fedik - edited - 1 Jul 2023
avatar Fedik
Fedik - comment - 11 Jul 2023

I have opened an issue in Codemirror repository for folding issue codemirror/dev#1206

avatar Fedik Fedik - change - 11 Jul 2023
The description was changed
avatar Fedik Fedik - edited - 11 Jul 2023
avatar Fedik
Fedik - comment - 11 Jul 2023

Should use php({baseLanguage: html({selfClosingTags: true}).language})

avatar Fedik Fedik - change - 15 Jul 2023
The description was changed
avatar Fedik Fedik - edited - 15 Jul 2023
avatar Fedik
Fedik - comment - 15 Jul 2023

I think it is good now

avatar Fedik Fedik - change - 15 Jul 2023
The description was changed
avatar Fedik Fedik - edited - 15 Jul 2023
avatar HLeithner
HLeithner - comment - 17 Jul 2023

@brianteeman can you test this please?

avatar brianteeman
brianteeman - comment - 17 Jul 2023

When you are using codemirror as the content editor and go to full screen mode and then try to use one of the xtd-editor buttons then you get stuck in a modal/backdrop loop

avatar brianteeman
brianteeman - comment - 17 Jul 2023

When using codemirror in the template manager if you toggle the "Show Original File" then you get a js error in the console


admin-template-toggle-switch.min.js?ac1f2cda16f61510b1b3bca453fe1fcf:1 Uncaught TypeError: Joomla.editors.instances.jform_core.refresh is not a function
    at HTMLInputElement.n (admin-template-toggle-switch.min.js?ac1f2cda16f61510b1b3bca453fe1fcf:1:1079)
n @ admin-template-toggle-switch.min.js?ac1f2cda16f61510b1b3bca453fe1fcf:1

avatar Fedik
Fedik - comment - 17 Jul 2023

... then you get stuck in a modal/backdrop loop

yeah, it a bootstrap "bug", it does not allow modal in container with fixed position...
Need to look, in past there was no xtd-buttons when codemirror in full screen, but I think it is nice to have.

avatar Fedik
Fedik - comment - 17 Jul 2023

The bugs is fixed, should work now, the modal and "Show Original File"

avatar brianteeman brianteeman - test_item - 17 Jul 2023 - Tested successfully
avatar brianteeman
brianteeman - comment - 17 Jul 2023

I have tested this item successfully on a8f84f2


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/41070.

avatar HLeithner HLeithner - close - 17 Jul 2023
avatar HLeithner HLeithner - merge - 17 Jul 2023
avatar HLeithner
HLeithner - comment - 17 Jul 2023

Thanks I'm merging this for now so we have it in alpha 3 and people can give some feedback.

avatar HLeithner HLeithner - change - 17 Jul 2023
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2023-07-17 12:56:25
Closed_By HLeithner

Add a Comment

Login with GitHub to post a comment