Skip to content
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

TinyMCE support for inline multi-language tags #11367

Closed
wants to merge 4 commits into from
Closed

TinyMCE support for inline multi-language tags #11367

wants to merge 4 commits into from

Conversation

dgrammatiko
Copy link
Contributor

@dgrammatiko dgrammatiko commented Jul 30, 2016

Pull Request for Issue joomla/accessibility#2
a redo of #11366

Summary of Changes

Support for <span lang="en" dir="ltr" xml:lang="en" depending on the language selected on the dropdown button.

Testing Instructions

Apply patch.
Make sure that you have more than one language installed in joomla, either wise you won't see something new :)

Select some text and then select the required language from the language button (it's a drop down)

Inspect the selected text to make sure that the text is wrapped with the span and the language tag.

Toggle the editor to inspect the actual html code, re toggle few more times to ensure that changes persist.

Save and see if changes are retained. Right now this seems to fail, it must be some weird php side filtering.

Preview

screen shot 2016-07-30 at 17 47 07

@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators PR-staging labels Jul 30, 2016
case 1:
default: /* Advanced mode*/
$toolbar1 = "bold italic underline strikethrough | alignleft aligncenter alignright alignjustify | formatselect | bullist numlist "
. "| outdent indent | undo redo | link unlink anchor image code | hr table | subscript superscript | charmap langs";
Copy link
Contributor

Choose a reason for hiding this comment

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

is it a mistake putting the image link back in here. it was deliberately removed so that we only have the one image link (the xtd-editor)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess that came from the copy paste, will undo

@brianteeman
Copy link
Contributor

I have tested this item 🔴 unsuccessfully on d42c1fe

I installed hebrew language
Created a content item and pasted in some hebrew - highlighted it and the correct span was applied
But when I saved the content item none of the content was saved


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

@dgrammatiko
Copy link
Contributor Author

@brianteeman the tinymce needs to know that span is allowed element, but I think this also fails in php validation (setting valid elements to *[*] and toggling the editor seems to work, but saving still strips the span element)

@brianteeman
Copy link
Contributor

span isnt the issue. you can enter a span with tinymce by default with no changes. I tested it with

< span class="b">test


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

@dgrammatiko
Copy link
Contributor Author

dgrammatiko commented Jul 30, 2016

@brianteeman weird, for me <span class="b">test</span> gets trimmed (not in tinyMCE but in PHP side)

@brianteeman
Copy link
Contributor

I should have said that I was testing the span without the patch


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

1 similar comment
@brianteeman
Copy link
Contributor

I should have said that I was testing the span without the patch


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

@brianteeman
Copy link
Contributor

Still no good. Made a movie

tinymce

@MATsxm
Copy link

MATsxm commented Jul 30, 2016

I have tested this item 🔴 unsuccessfully on 3f09c54

At this point and after applying the patch.

@dgrammatiko
Copy link
Contributor Author

@MATsxm now you should be able to see the button, my bad

@MATsxm
Copy link

MATsxm commented Jul 31, 2016

@dgt41

Ok, the BTN is back BUT as soon as the patch is applied (com_patchtester), then I'm not abale to save any content.

Hom to reproduce:

  • Apply the patch
  • Create a new article
  • Add "test" in the content box (html or not)
  • Click 'SAVE'
  • The input "test" disappear and is not saved

Revert the patch, then it works as expected


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

@brianteeman
Copy link
Contributor

I have tested this item 🔴 unsuccessfully on d90b9b1

Test on a new article - press save (with or without applying the langauage tag) nothing is saved
Test on an existing article - edit text (with or without applying the langauage tag) - press save content reverts to the saved version

Apply the language tag to some content and check the code created and it is
<p><span lang="xml" dir="ltr">dvdsvks</span></p>

Note the incorrect tags


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

@dgrammatiko
Copy link
Contributor Author

@MATsxm @brianteeman would be fine now

@dgrammatiko
Copy link
Contributor Author

One thing that I couldn;t figure out is the usage of the icon \e907 screen shot 2016-07-31 at 13 20 42

@brianteeman
Copy link
Contributor

brianteeman commented Jul 31, 2016 via email

@MATsxm
Copy link

MATsxm commented Jul 31, 2016

of course 907 would be the one

@brianteeman
Copy link
Contributor

brianteeman commented Jul 31, 2016 via email

@MATsxm
Copy link

MATsxm commented Jul 31, 2016

On the JDoc (and AFAIR for other projects) we use it like it is with toolTips
sans titre
(note, the "lang" and "C" dropdowns are for other kind of purpose)
I guess @brianteeman is the one to find the perfect "caption".

@dgrammatiko
Copy link
Contributor Author

dgrammatiko commented Jul 31, 2016

The conflict comes from .mce-ico which points to font-family: 'tinymce-small', Arial instead of font-family: 'tinymce', Arial. So it should be something with the small buttons logic.
Nope, there is a bug in skin.min.css:
.mce-btn-small .mce-ico{font-family:'tinymce-small',Arial}
should be
.mce-btn-small.mce-ico{font-family:'tinymce-small',Arial}
No space between the classes

@dgrammatiko
Copy link
Contributor Author

Issue on their repo: tinymce/tinymce#3095

@MATsxm
Copy link

MATsxm commented Jul 31, 2016

Not talking about the icon.

Test1: creating a new article with content -> save (OK) -> using the "Language span" feature -> save then OK with: <p><span dir="ltr" lang="fr" xml:lang="fr">test</span></p>

Test2: creating a new article with content -> using the "Language span" feature -> save then OK with: <p><span dir="ltr" lang="fr" xml:lang="fr">test2</span></p>

Test3: Edit an existing en-GB article with content -> using the "Language span" feature -> save then OK with: <span dir="ltr" lang="fr" xml:lang="fr">incorrupte definitionem</span>

Note: no other languages available than those installed.


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

@dgrammatiko
Copy link
Contributor Author

@MATsxm so apart from the icon the rest works as expected?

@MATsxm
Copy link

MATsxm commented Jul 31, 2016

IMHO Yup!

<span dir="ltr" lang="fr" xml:lang="fr">**MERCI!**</span>
😉

@brianteeman
Copy link
Contributor

@dgt41 did you try with the 4.4.1 update #11367

@dgrammatiko
Copy link
Contributor Author

dgrammatiko commented Jul 31, 2016

@brianteeman will try that. tinyMCE 4.4.1 solves the icon problem (theme.min.js)

For the next step here I will need some help. I got the list of all the known languages in the globe from here: http://stackoverflow.com/questions/3217492/list-of-language-codes-in-yaml-or-json
I added the missing direction (but is ltd for all of them) and now somehow we need to

  • Verify the language codes
  • Verify the Name
  • Verify the translated Name
  • Assign correct direction fro the RTL ones

The gist for the json data is here: https://gist.github.com/dgt41/c18160606be9ca963b54c5b91ecc9e9f

Once the data is verified the backend of tinyMCE will become like:
screen shot 2016-07-31 at 21 47 20

The admin could pre-select the available languages for the dropdown.
Help here would be greatly appreciated!

@brianteeman
Copy link
Contributor

brianteeman commented Jul 31, 2016 via email

@dgrammatiko
Copy link
Contributor Author

I don't mind the source, the end point should be either an array or a JSON with code, name in English, name in local language and direction. (or at least this is my best guess)

@MATsxm
Copy link

MATsxm commented Jul 31, 2016

@dgt41
I'm also working on a new list: https://github.com/MATsxm/l10n-list/blob/master/php.php
Still many things to do, but can be a good starting point

$langDir = $availLang['dir'];
$langsConstructorTmp[] = '
{ text: "' . $langName . '",
onclick: function () {
Copy link
Member

Choose a reason for hiding this comment

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

sorry, but it looks bad 😄
check how easy to make custom plugin for TinyMCE https://github.com/tinymce/tinymce/blob/master/js/tinymce/plugins/hr/plugin.js

// add button options in PHP
$scriptOptions['joomlalanguages'] = array(
 'lang'=> $langTitle,
 'dir' => $langDir
 .... more
);

// enable button
$toolbar1 .= ' joomlalanguages';
// Custom TinyMCE plugin "joomlalanguages"
tinymce.PluginManager.add('joomlalanguages', function(editor) {

   editor.addCommand('doSomeLanguageStuff', function() {
    var setings = editor.setings.joomlalanguages || {};
    // here is all code for do fancy stuff
  });

    editor.addButton('joomlalanguages', {
        icon: 'translate',
        tooltip: Joomla.JText('PLG_TINY_LANGUAGES_LABEL'),
        cmd: 'doSomeLanguageStuff'
    });
});

you can place that JS in tinymce-init.js at the end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it doesn't use the tinymce-init.js ATM, I've started this before this get merged. But I will change it...

Copy link
Member

Choose a reason for hiding this comment

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

tinymce-init.js was as example, you can even make own plugin file, depend what you liked more 😉

Copy link
Member

Choose a reason for hiding this comment

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

then it will be more easy with #11926, will be enough just add joomlalanguages to preset, and to the Known Buttons, in theory

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Fedik I think I will wait till your PR gets merged, before I'll redo this one!

$languageButton = 'JlanguagesBtn';

JText::script('PLG_TINY_LANGUAGES_LABEL');
$pluginFile = JHtml::_('script', 'editors/tinymce/plugin-joomlalanguages.js', false, true, true);
Copy link
Contributor

Choose a reason for hiding this comment

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

can we use the new method signature and add the script version?

JHtml::_('script', 'editors/tinymce/plugin-joomlalanguages.js', array('version' => 'auto', 'relative' => true, 'pathOnly' => true));

See https://github.com/joomla/joomla-cms/pull/12373/files for more info

dgrammatiko and others added 3 commits October 27, 2016 06:49
commit e17e53d
Author: dgt41 <d.grammatiko@gmail.com>
Date:   Mon Aug 8 01:37:54 2016 +0300

    cs

commit 8513cb5
Author: dgt41 <d.grammatiko@gmail.com>
Date:   Mon Aug 8 01:15:26 2016 +0300

    CS

commit 7194fa0
Author: dgt41 <d.grammatiko@gmail.com>
Date:   Mon Aug 8 00:50:36 2016 +0300

    drop json, probably good idea only if it was used

    directly into tinyMCE

commit 351e0dc
Author: dgt41 <d.grammatiko@gmail.com>
Date:   Sun Aug 7 23:42:33 2016 +0300

    naming

commit 144790f
Author: Dimitri Grammatikogianni <mit505@upshift.gr>
Date:   Sun Aug 7 22:36:25 2016 +0300

    json from @MATsmx

commit d321ead
Author: dgt41 <d.grammatiko@gmail.com>
Date:   Sun Aug 7 21:39:35 2016 +0300

    fixes

commit 7980883
Author: dgt41 <d.grammatiko@gmail.com>
Date:   Sun Aug 7 20:36:36 2016 +0300

    strings

commit 9c11a79
Author: dgt41 <d.grammatiko@gmail.com>
Date:   Fri Aug 5 10:27:38 2016 +0300

    fix php notice

commit dac5c3a
Author: dgt41 <d.grammatiko@gmail.com>
Date:   Fri Aug 5 10:22:23 2016 +0300

    CS

commit 012fb12
Author: dgt41 <d.grammatiko@gmail.com>
Date:   Mon Aug 1 10:14:32 2016 +0300

    fields for selecting languages

    - fix mobile mode

commit b3be294
Author: dgt41 <d.grammatiko@gmail.com>
Date:   Sun Jul 31 14:37:41 2016 +0300

    icon

commit bcb47d4
Author: dgt41 <d.grammatiko@gmail.com>
Date:   Sun Jul 31 13:25:47 2016 +0300

    don't lookup for a button if it doesn't exist

commit 294aa27
Author: dgt41 <d.grammatiko@gmail.com>
Date:   Sun Jul 31 13:18:34 2016 +0300

    span check fix

commit 62c8b57
Author: dgt41 <d.grammatiko@gmail.com>
Date:   Sun Jul 31 12:53:49 2016 +0300

    fix save problem, wrong tag values

commit c46b487
Author: dgt41 <d.grammatiko@gmail.com>
Date:   Sun Jul 31 03:22:24 2016 +0300

    more

commit 211552d
Author: dgt41 <d.grammatiko@gmail.com>
Date:   Sun Jul 31 02:09:03 2016 +0300

    icon, fixes

commit 60721e0
Author: dgt41 <d.grammatiko@gmail.com>
Date:   Sat Jul 30 22:36:10 2016 +0300

    cs

commit 428fdc5
Author: dgt41 <d.grammatiko@gmail.com>
Date:   Sat Jul 30 22:04:48 2016 +0300

    CS

commit de41673
Author: dgt41 <d.grammatiko@gmail.com>
Date:   Sat Jul 30 21:46:20 2016 +0300

    remove duplicate image icon

commit d7b43c6
Author: dgt41 <d.grammatiko@gmail.com>
Date:   Sat Jul 30 21:43:01 2016 +0300

    CS

commit 37dcf6b
Author: dgt41 <d.grammatiko@gmail.com>
Date:   Sat Jul 30 21:40:16 2016 +0300

    fix case that selected text is already in a span

commit edbc86a
Author: dgt41 <d.grammatiko@gmail.com>
Date:   Sat Jul 30 20:50:06 2016 +0300

    Allow span

commit 73c7106
Author: dgt41 <d.grammatiko@gmail.com>
Date:   Sat Jul 30 19:47:30 2016 +0300

    CS

commit 590f0dd
Author: dgt41 <d.grammatiko@gmail.com>
Date:   Sat Jul 30 19:33:39 2016 +0300

    languages tags
@dgrammatiko dgrammatiko changed the base branch from 3.7.x to staging October 27, 2016 03:52
@joomla-cms-bot joomla-cms-bot removed the Language Change This is for Translators label Nov 14, 2016
@zero-24 zero-24 removed the Feature label Nov 14, 2016
@dgrammatiko dgrammatiko reopened this Nov 21, 2016
@joomla-cms-bot joomla-cms-bot added the Language Change This is for Translators label Nov 21, 2016
@dgrammatiko
Copy link
Contributor Author

All known and reported issues should be fixed now. Please test!


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

@brianteeman
Copy link
Contributor

Just applied the PR but no languages button on the toolbar
Checked the plugin and its set to enabled
Then I saw the field to select the language and once I had selected some the languages button appeared.
So

  1. Can you update the test instructions in the first post with that info
  2. Do you think it would be more user friendly if by default all installed content languages are available (and you can add others if needed in the plugin)

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

@brianteeman
Copy link
Contributor

Sorry to say but it is still not correctly applying the tag to the correct
In this video I simulated selecting a phrase inside a paragraph and setting the lang on that phrase as french.
As you can see that didnt work as intended

screen shot 2016-11-21 at 16 35 45

.


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

@brianteeman
Copy link
Contributor

Another simple test of a hebrew RTL phrase inside an english article
screen shot 2016-11-21 at 16 41 45


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

@brianteeman
Copy link
Contributor

For reference it is expected according to the spec to have a pharse in language B inside a paragraph written in language A https://www.w3.org/International/questions/qa-html-language-declarations#usingspan


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

@dgrammatiko
Copy link
Contributor Author

dgrammatiko commented Nov 21, 2016

@brianteeman thanks! So this is a tough one, as solving one problem creates problems for other cases...

Please don't test yet, WAIT for an update here!

@dgrammatiko
Copy link
Contributor Author

I will reopen this one as soon as I get some free time...

@dgrammatiko dgrammatiko deleted the tinyLangsTags branch July 22, 2017 20:20
@brianteeman
Copy link
Contributor

@dgrammatiko I think I have a solution for this if you are able to assist

@dgrammatiko
Copy link
Contributor Author

@brianteeman sure, just FYI I rechecked the code here and it's missing a bit of logic to work as it was expected

@brianteeman
Copy link
Contributor

This is part of the EU funded research project for improving the process of creating accessible content by authors https://accessibilitycluster.com/about .I am representing JandBeyond EV on this project (politics prevented it saying joomla or joomla promoting it)

The aim of the project is mainly in researching and identifying key needs and areas where all cms can benefit. The project might eventually produce some code examples but that's not the main aim.

The research conducted in stage one of the project identified numerous needs by content authors and we are working on testing and evaluating the possibilities for improving each one of those. Not all of them apply to Joomla or we already address those issues.

I am currently reviewing the identified needs to see how they fit with Joomla. I think I have the basis of a solution to this one and then saw your PR from the stone ages ;)

Tinmyce natively has buttons now to se the direction of the language but it still doesnt have anything to mark the language. This is essential to satisfy the "language of parts" accessibility rule. WCAG 2.0: Understanding 3.1.2 Language of Parts: http://www.w3.org/WAI/GL/UNDERSTANDING-WCAG20/meaning-other-lang-id.html

Most other CMS already have a solution for this - just not joomla

I came across this native tinymce solution provided by a third party https://github.com/edx/tinymce-language-selector/

Would you be able to see about integrating that. JS is beyond me ;)

Hope that clarifies things a little.

@dgrammatiko
Copy link
Contributor Author

@brianteeman the code of that plugin needs transpilling, here's a version that will work with the current Joomla's tinyMCE: https://gist.github.com/dgrammatiko/8631faa0ea083e139a7e36e41690b1b6

About reviving this PR: I was looking on that plugin and it seems that quite a lot of what we did here is useless (everything is done in the js land with that plugin). I guess you can open a PR with the switch and the needed changes in the allowed tags and I will push the JS changes there. I mean there's not much that we can take from this one...

@brianteeman
Copy link
Contributor

Thanks. I didnt expect there would be much if anything we could take from here as its so old and for an earlier version.

Is the transpiling something we can automate?

Will check more closely abd create a draft Pr hopefully tomorrow

@dgrammatiko
Copy link
Contributor Author

Is the transpiling something we can automate?

The J4 build tools supposed to do that (I haven't tried if any imports are actually resolved). But anyways it's something that is patchable

@brianteeman
Copy link
Contributor

great - thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Language Change This is for Translators
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants