-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
[4.1][POC][RFC] Make tinyMCE a true WYSIWYG editor #35669
Conversation
I have no idea how you can possibly approve this PR so quickly. There are a LOT of changes that need to be tested. In particular the whole idea of the child templates which I am yet to see to be able to comment fully on. When this PR is finished I will test it. |
@brianteeman this is a POC so basically it's down to the maintainers to respond with an approval or rejection or something in between. The fact that I provided actual working code instead of pseudo here was just to help people with the decision (fewer things to imagine). I don't expect anyone to test this PR at this state... Edit, added also an RFC tag, to help anyone that wants to contribute their ideas here
there's a Cassiopeia clone here that is following all the requirements for the new mode (child templates): |
@brianteeman I approve of that little change I commented about, not the entire commit. I probably shouldn't play with those buttons :) I'm going to dig into the entire commit as soon as I get some strength. I would probably try and hook into the branch itself and do some contributions myself. I think so far the direction is going somewhere and this time we might actually have some results. I'm interested in having a TinyMCE instance specific to each template/module in the sense that:
Also I'm interested in overriding the entire stuff: TinyMCE javascript init, plugins, toolbars, templates, everything. Sounds good? |
This is already possible at the moment. create a plugin listening for |
I know I can create an entire plugin basically a JCE 2.0, what I'm interested is to do that without a system plugin or anything, just using the right structure + code for templates and modules WITH the default Joomla TinyMCE editor plugin. This way end-users have less options to dig in order to enable the editor the developer designed. How cool is that: no need for page builders, hacks or bloatware. |
Exactly what I'm proposing here. Let me explain with a simple use case what's happening (thanks to child templates and the existing architecture which I'm using here):
|
My site has two templates. What happens now with the editor? The template doesn't get associated with the content until after it is created. |
There are 2 possible outcomes here:
|
|
||
'plugins' => implode(',', array_unique($plugins)), | ||
'plugins' => implode(',', array_unique($this->plugins)), | ||
|
||
// Quickbars | ||
'quickbars_image_toolbar' => false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really hope this gets changed to TRUE and to have align-left align-center align right | edit delete
options, where edit is done via the media manager and not the TinyMCE useless pop-up. Something like this could do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is just a flip of the boolean. I have no clue why it's false by default but probably you can PR the change (not in this PR obviously as a separate own PR)
Why wouldnt it be a completely different with a completely different design, structure and even framework. And no, editing in the front end doesn't necessarily use the default template and you dont address creating content in either site or admin. Honestly I really dont like the way this sounds like its going |
So, what I'm proposing covers 99% of the users. One site one template. This makes it extremely intuitive for all these users, they don't have to do anything (as long as they pick the right template that supports these features). Also, as I said child templates are here to help people deliver more designs using one template. The necessity for multiple templates should be eradicated by the child templates (for almost all cases). About the 1% that needs something else they can:
|
thats the part that concerns me |
There are some missing switches atm that will override the behaviour (use the defaults), maybe that part makes you think that these are dictated, the user still has control over all aspects (on/off, custom overrides). That said we can educate users, eg have a very good explanation of what |
The tinymce templates/snippets were deliberately placed in the /templates folder so that they can be created and edited in the template manager. Unless I am missing something moving them to the /media folder prevents that. |
It can be today although I personally just include the typography related css |
Am I missing something. I have applied the PR, npm i, nstalled cassy, made that default. But when I try to create any content in the site or admin I get the following erors
|
At the moment Child templates aren't supported in the UI of Joomla but once #32896 is done (which is the prerequisite for enabling child templates) this will be possible. One of the fundamental changes for the child templates is that they should be safer (eg no public access to
Placing some STATIC CSS in a file will never be able to accurately represent the styles of a template due to the simple fact that almost all templates allow some kind of colour selection that is stored in the db. By definition this customization cannot be somehow done with static CSS without the need of constantly changing values in multiple places (the template form UI, and the editor.css). This approach just eliminates that, and can still read the editor.css and include that in the response (not sure if I already coded that part right now)
I probably messed up something. I will check this later on today |
That suggests that they would only work then for a template that has implemented child templates? I would argue that these content templates/snippets are NOT static assets at all. Without being able to test it at all then I can't really make any more constructive comment |
No, existing templates will continue to expect the
They are, the definition is coming from the way you are using them. They are fetched DIRECTLY from the browser thus for Joomla should be considered static assets. In the Joomla relm static is anything that is not served through PHP.
Will try to fix it in a bit |
thats why I replied asap ;) |
@brianteeman I've updated the front end template cassy_1.0.0.zip |
No errors now and I can test it. |
probably beyond the scope but cassy is missing var(--cassy-color-primary) |
Here's abetter version: Updated `jeditor.php`<?php
/**
* @package Joomla.Site
* @subpackage Templates.cassy
*
* @copyright (C) 2021 Open Source Matters, Inc. <https://www.joomla.org>
* @license GNU General Public License version 2 or later; see LICENSE.txt
*/
defined('_JEXEC') or die;
/** @var Joomla\CMS\Document\HtmlDocument $this */
/**
* This endpoint returns the CSS content for the template
*/
$this->setMimeEncoding('text/css');
$this->setCharset('utf-8');
$wa = $this->getWebAssetManager();
$mv = $this->getMediaVersion();
$fontStyles = '';
$editorCSS = '';
// Use a font scheme if set in the template style options
$paramsFontScheme = $this->params->get('useFontScheme', false);
if ($paramsFontScheme) {
if (stripos($paramsFontScheme, 'https://') === 0) {
$this->getPreloadManager()->preconnect('https://fonts.googleapis.com/', []);
$this->getPreloadManager()->preconnect('https://fonts.gstatic.com/', []);
$wa->registerAndUseStyle('fontscheme.current', $paramsFontScheme, [], []);
if (preg_match_all('/family=([^?:]*):/i', $paramsFontScheme, $matches) > 0) {
$fontStyles = '--cassy-font-family-body: "' . str_replace('+', ' ', $matches[1][0]) . '", sans-serif;
--cassy-font-family-headings: "' . str_replace('+', ' ', isset($matches[1][1]) ? $matches[1][1] : $matches[1][0]) . '", sans-serif;
--cassy-font-weight-normal: 400;
--cassy-font-weight-headings: 700;';
}
} else {
$wa->registerAndUseStyle('fontscheme.current', $paramsFontScheme, ['version' => 'auto'], []);
}
}
// Color Theme
$paramsColorName = $this->params->get('colorName', 'colors_standard');
$wa->registerAndUseStyle('theme.' . $paramsColorName, 'media/templates/site/cassy/css/global/' . $paramsColorName . '.css');
// Enable assets
$wa->useStyle('template.cassy.' . ($this->direction === 'rtl' ? 'rtl' : 'ltr'))
->useStyle('template.active.language')
->useStyle('template.user');
// Get the URLs
$templateCSSUri = $wa->getAsset('style', 'template.cassy.' . ($this->direction === 'rtl' ? 'rtl' : 'ltr'))->getUri();
$templateColorsUri = $wa->getAsset('style', 'theme.' . $paramsColorName)->getUri();
$activeLanguageCSSUri = $wa->getAsset('style', 'template.active.language')->getUri();
$userCSSUri = $wa->getAsset('style', 'template.user')->getUri();
$fontsCSSUri = $wa->assetExists('style', 'fontscheme.current') ? $wa->getAsset('style', 'fontscheme.current')->getUri() : '';
// Assign the URLs to CSS imports
$templateCSS = ($templateCSSUri !== '') ? '@import url("' . $templateCSSUri . '?' . $mv . '");' : '';
$templateColorsCSS = ($templateColorsUri !== '') ? '@import url("' . $templateColorsUri . '?' . $mv . '");' : '';
$activeLanguageCSS = ($activeLanguageCSSUri !== '') ? '@import url("' . $activeLanguageCSSUri . '?' . $mv . '");' : '';
$userCSS = ($userCSSUri !== '') ? '@import url("' . $userCSSUri . '?' . $mv . '");' : '';
$fontsCSS = ($fontsCSSUri !== '') ? '@import url("' . (strpos($fontsCSSUri, '/') === 0 ? $fontsCSSUri . '?' . $mv : $fontsCSSUri) .'");' : '';
// The user editor.css from /media/templates/site/cassy/css/editor.css
if (is_file(JPATH_ROOT . '/media/templates/site/cassy/css/editor.css')) {
$editorCSS = @file_get_contents(JPATH_ROOT . '/media/templates/site/cassy/css/editor.css');
}
echo <<<CSS
@charset "UTF-8";
/** Template CSS */
$templateCSS
/** Template Colours */
$templateColorsCSS
/** Active Language CSS */
$activeLanguageCSS
/** User CSS */
$userCSS
/** Fonts CSS */
$fontsCSS
/** Inline */
:root {
--hue: 214;
--template-bg-light: #f0f4fb;
--template-text-dark: #495057;
--template-text-light: #ffffff;
--template-link-color: #2a69b8;
--template-special-color: #001B4C;
$fontStyles
}
/* Editor styles from css/editor.css */
$editorCSS
CSS; |
I can see how it works now ;) Question 1 |
The quick answer is YES, but probably it's the wrong one as well. The defaults already exist in the tinyMCE's internal configuration and yes we can combine merge JSON files with ease. The question that needs to be addressed is what consists of the base JSON that the templates will extend? A reminder that some classes might be CSS Framework specific. A few years back when @ciaran and I were proposing a Joomla CSS Framework and that was because we believed (I still do) that some classes should be owned by the CMS and anyone that implements a Bootstrap, Tailwind, whatever template has to extend their code to cover these classes. Right now there's no common denominator so it's kinda hard to come up with a base JSON that covers everybody. But maybe I'm wrong here... EDIT: The project could invest into a static site that could generate those JSON files in a gui fashion |
So my other concern is that the jeditor.php file is very complex (at least for me to understand) |
That's more a Cassiopeia related issue, in short the jeditor.php is something like: defined('_JEXEC') or die;
/** @var Joomla\CMS\Document\HtmlDocument $this */
/** Set the right mime/encoding */
$this->setMimeEncoding('text/css');
$this->setCharset('utf-8');
/** Concatenate and inline all the CSS parts that served in the normal template */
$css = '....';
/* retrun the inlined CSS */
echo $css; So all the complexity is from the Web Assets, Custom colours, Custom Fonts, etc that are specific to Cassiopeia and the way these were implemented. Another template might have 1/3 of that logic or 4x more code, the sky is the limit... |
just wanted to say that I get the benefits of this now. Took me a while - sorry. |
I'm happy my issue #35606 has such a successful path. I'm happy to come here for a test as soon as I can. |
I get the benefit of this so much more now. |
@brianteeman I’m happy to finalize this if it ever gets an approval but I’m also happy that you already did the container part. Btw the tinymce’s docs have some more there: https://www.tiny.cloud/docs/demo/format-html5/ |
I forgot you had done this and started to do the same exact thing this morning after feedback from my "do more with core" talk last night. |
This one could be revisited in v5 |
I'll get back to this as soon as I can, unfortunately couldn't assist lately. Why close? |
@dgrammatiko I'm the one who wrote the code, not you, so please don't be upset and close your PRs :) |
I don't share that guy's opinion, if he wants J3 functionality to be forever, he should probably stick to J3. I think your PR here is great without even testing it, I've read some code and I like it. I'm sure I'm gonna love it once I've actually tested it :) So please take it easy man, I'm here. |
@joomdonation I'm not gonna close the open PRs that are still valid and I'm also gonna finish the child templates work as I promised because today apparently it's #WorldKindnessDay @thednp parts of this PR are already implemented with the child templates and also @brianteeman did the base for the JSON in TinyMCE with #35715 so things could at some point reach the point I demoed here. The editor CSS endpoint didn't make it for 4.1, but you could try again in a future release. |
@dgrammatiko Happy to hear that and thanks ! |
Please reopen the PR, I'm going to work on it next on my list. I don't see any reason why this wouldn't make it to 4.1. |
@thednp as I said before some parts are already part of other PRs. If you want to work on the remaining parts these are:
You're more than welcome to use any of the code here, eg
That said please get some advice either from @bembelimen (4.1) or @HLeithner, I think the features for 4.1 are already fixed (but I might be wrong) |
Don't allow yourself be bullied by ignorant fools |
- simplify the logic for the editor.css - add the code from joomla#35669 for the template snippets to support files placed in the media folder (aka publically accessible)
Pull Request for Issue #35606 and partially a redo of #14456
Summary of Changes
Summary, testing instructions, etc will be updated later today or tomorrow (I need to finish the templates for the testing as the current templates are not supporting childs and the changes are available only for those)
This PR is a proposal for 4 changes:
style_formats.json
andformats.json
to allow tinyMCE to use Bootstrap, Tailwind or whatever other CSS frameworks in the produced HTMLSo, it seems that I'm trying to revert a very bad implementation of the skins that was introduced by... (drum roll) me back in 2015. Why is wrong? First because it expects the final user to decide on the looks of TinyMCE when that decision is not up to them but it's up to the TEMPLATE. The solution is almost a one line
HTMLHelper::stylesheet()
that will load the provided from tinyMCE skin or will be overrided if in the templates foldercss/plg_editor_tinymce/skins/oxide/...
exists. Simple, elegant, intuitive. Install a backend/frontend template, if it supports custom skin to match the tinyMCE with the rest of the template's design will be used, if not the default is there...When Brian reworked the templates part of tinyMCE he used the template's html folder for storing the files under a folder name
tinymce
. This is fine for the existing templates but will not play well with the templates implementing the new mode. The reason is simple the new mode calls for all static assets to be served from the folder/media/templates/...
so that the actual templates folder will hold only PHP related files and thus could be locked and not being an entry point for the site (Joomla's entry points are:/index.php
, '/installation/index.php', '/administrator/index.php', '/administrator/components/com_joomlaupdate/update.php', '/media', '/images' and the new one for api/api/index.php
, meaning that anything else should be locked from direct access to a request). This introduces a minor change, legacy templates will have the tinyMCE templates in the/templates/templateName/html/tinymce
while the ones supporting the new mode will have to get those files in/media/templates/site/templateName/html/tinymce
. This would need to be documented when the docs for the child templates will be written.This might be controversial but the editor.css that tinyMCE is using IMHO should be an exact copy of the front end template CSS. Since Joomla has a pretty nice way for doing this (
?tmpl=something
will load thesomething.php
in the template, I build an actual PWA that composes the pages in the service worker just by abusing this https://github.com/dgrammatiko/sloth ) the proposal is to standardize an endpoint for the new mode templates. We don't break anything and we bring WYSIWYG in a opt in way.sample implementation of jeditor.php
Well, this is a redo of #14456. TinyMCE has a way to be customised with couple of json files. In short the menu elements and the toolbar buttons whenever clicked are runing a function and that function can be customised from the json files. Eg the menu
Format->Align Left
and the buttonAlign Left
by default wrapping the selected text to a span with an inline styletext-lign: left
. In the JSON file we can change this to match our frontend template and produce<span class="text-start">
These are very well documented:
style_formats.json
formats.json
Testing Instructions
You will need to apply this PR and also install 2 templates: Cassy a Cassiopeia clone that supports child templates
cassy_1.0.0.zip and admin2021 a clone of Atum again ported to support childs
Actual result BEFORE applying this Pull Request
Expected result AFTER applying this Pull Request
Documentation Changes Required
@thednp @brianteeman
@ciar4n @kawshar could you please provide some feedback here? Is this something that you would support in your templates?