? ? Success
Related to # 7152

User tests: Successful: Unsuccessful:

avatar dgt41
dgt41
13 Jun 2015

Render the editors-xtd buttons as native tinyMCE buttons

So what is this all about?
Well Joomla has those 5 buttons bellow tinyMCE which are not so helpful and also are tightly related to mootools. With this PR we try to get this buttons to render as native tinyMCE buttons.

Preview

screen shot 2015-06-14 at 18 59 37
screen shot 2015-06-14 at 15 59 44

avatar dgt41 dgt41 - open - 13 Jun 2015
avatar joomla-cms-bot joomla-cms-bot - change - 13 Jun 2015
Labels Added: ?
avatar joomla-cms-bot joomla-cms-bot - change - 13 Jun 2015
Labels Added: ?
avatar dgt41 dgt41 - change - 13 Jun 2015
The description was changed
avatar zero-24 zero-24 - change - 14 Jun 2015
Status New Pending
Easy No Yes
avatar zero-24 zero-24 - change - 14 Jun 2015
Category JavaScript Plugins
avatar zero-24 zero-24 - change - 14 Jun 2015
Rel_Number 0 7152
Relation Type Related to
avatar dgt41
dgt41 - comment - 14 Jun 2015

@nikosdion Can you take a look on this one?

avatar dgt41 dgt41 - change - 14 Jun 2015
The description was changed
avatar nikosdion
nikosdion - comment - 14 Jun 2015

Yup, that was what I was talking about at JaB 15 :)

avatar dgt41 dgt41 - change - 14 Jun 2015
The description was changed
avatar dgt41 dgt41 - change - 15 Jun 2015
The description was changed
avatar nikosdion
nikosdion - comment - 15 Jun 2015

Yes, there's a better way which is compatible with 3PD plugins.

The editor-xtd plugins should be able to define many icons, separated by comma e.g. "arrow-down pagebreak". However, this is not a very good solution as it requires knowledge of how the editor rendering the buttons work which is not fully abstracted.

The other way would be using the Bootstrap icon classes instead of images in TinyMCE. It seems to be possible but I'll let you decide if it's feasible – my knowledge of TinyMCE's advanced features is superficial at best.

avatar dgt41
dgt41 - comment - 15 Jun 2015

@nikosdion I will follow a different path on this one, commit later today/tonight...

avatar dgt41
dgt41 - comment - 15 Jun 2015

@nikosdion how about this?

Also my previous comment wasn’t about icons but for this preg thing that I am not really good at
https://github.com/dgt41/joomla-cms/blob/__tiny_btns2/plugins/editors/tinymce/tinymce.php#L636-L641
any idea how to get this in two lines?

avatar dgt41 dgt41 - change - 15 Jun 2015
Title
Editor xtd buttons as native tinyMCE plugins TAKE 2
[New feature] Editor xtd buttons as native tinyMCE buttons
avatar dgt41 dgt41 - change - 15 Jun 2015
Title
Editor xtd buttons as native tinyMCE plugins TAKE 2
[New feature] Editor xtd buttons as native tinyMCE buttons
avatar zero-24 zero-24 - change - 15 Jun 2015
Labels Added: ?
avatar nikosdion
nikosdion - comment - 15 Jun 2015

I think the preg is correct. The best way to check is test it live with https://www.functions-online.com/preg_match.html

avatar dgt41
dgt41 - comment - 17 Jun 2015

@nikosdion JEditor needs some changes to incorporate the button in the initialization process. Right now what I am doing here is very inefficient since I call the buttons once on init() (and do some reseting on css and js) and once again onDisplay() where the css and js is correctly passed.
The thing is that onInit() is not caring the infos wanted to initialize the scripts there.
Any ideas?

avatar nikosdion
nikosdion - comment - 17 Jun 2015

Sorry, no ideas :(

avatar JoshuaLewis JoshuaLewis - test_item - 17 Jun 2015 - Tested successfully
avatar JoshuaLewis
JoshuaLewis - comment - 17 Jun 2015

I've successfully tested this and is working as expected (both frontend and backend). :-) It even rendered my custom text button which is not in the core of Joomla. Would love to see this in the future of Joomla! Having everything within a single editor area is very ideal for the user experience.


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

avatar brianteeman
brianteeman - comment - 18 Jun 2015

Looks great to me - in the toolbar when Tiny is the editor - kept as buttons when it is not

I just wish there was a way that 3pd buttons had to "opt-in" to this changed (and improved) behaviour


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

avatar brianteeman brianteeman - test_item - 18 Jun 2015 - Tested successfully
avatar nikosdion
nikosdion - comment - 18 Jun 2015

In theory we could have a different event for toolbar display (onDisplayToolbar instead of onDisplay). This means some extra work for 3PDs like me but it would also solve the conundrum with the toolbar icons @dgt41 is currently facing. As far as I'm concerned I'd rather spend all of 10 minutes enhancing each of my editor-xtd plugins and make them display in a fashion that makes sense for end users. After all we're writing the software for the end users – even though we tend to forget it.

avatar dgt41
dgt41 - comment - 18 Jun 2015

@nikosdion I think introducing a new event falls to v4 roadmap! I guess many people will object that this is a huge break of B/C blah blah...

avatar nikosdion
nikosdion - comment - 18 Jun 2015

It's not a b/c break because the old behaviour (onDisplay is used to display the buttons below the editor) still exists. There is now a new, optional behaviour (onDisplayToolbar). If you don't implement it, no problem, your button still shows below the editor. If you implement this optional event cool bananas, your button is rendered in the toolbar.

If you want it's even less involved than Tags and Content Versioning which were added in the middle of the 3.x release cycle. These features did have a b/c impact on templates: if they didn't implement overrides for the new components they broke. What I'm proposing is 100% backwards compatible in every possible respect.

avatar dgt41
dgt41 - comment - 18 Jun 2015

Stupid me, if you keep onDisplay as is of course no B/C ????

avatar brianteeman
brianteeman - comment - 18 Jun 2015

Thats pretty much what I was trying to explain/suggest on glip

avatar dgt41
dgt41 - comment - 18 Jun 2015

Let’s see the objective here:
1. Improve UX/UI
2. Remove the mootools from the buttons (at least for tinyMCE as this is the only editor affected here)

With the current implementation we achieve both. Win!
No, people might want the 3PD buttons bellow the editor. That ain’t gonna happen with this implementation because breaking the buttons inside/outside the editor we fall sort for the second if the 3PD don’t follow these changes.
I know, as I stated in the comments above, that this implementation is inefficient, since you execute one function twice for every editor but I think this is what is B/C doable at the moment (at least this is what I can think of).

To have a performance benchmark you car try to create a new article in isis with latest staging and then apply this together with #7167, #5871, #5655, #5654 and see the difference, if there is any ????.

avatar JoshuaLewis
JoshuaLewis - comment - 18 Jun 2015

@dgt41 Perhaps we could have some comment(s) in the code so that later down the road it will be more noticeable by developers to go back and make it more efficient. This way we can have a better user experience now and a performance booster later.

avatar dgt41
dgt41 - comment - 18 Jun 2015

@JoshuaLewis the problem is not the code in tinymce.php file but the way JEditor class is structured right now (we need to initialize twice the buttons, once to get the name, class, properties and another time to get the css and JS). The problem is that on Editor inititialazation (where we built the icon inside tinymce) we don’t have the values for author, the field where the editor will render and I am sure I forget some more.
On the code in tinymce.php there are comments on what is going on in every step and everyone can follow, nothing magic really here!

avatar Bakual
Bakual - comment - 18 Jun 2015

I don't think performance is a crucial topic when it comes to the editor. Pages using the editor are probably not the ones which are loaded multiple times in a second. Not an article page or category listing page which is more likely to get loaded very often.

avatar dgt41
dgt41 - comment - 20 Jun 2015

@Bakual I managed to do everything in one call, so no more twice calling the function getButtons()

The only thing right now is to decide if PLT wants to solve also the question @brianteeman asked above

Other than that this is good to go!

avatar Bakual
Bakual - comment - 20 Jun 2015

The only thing right now is to decide if PLT wants to solve also the question @brianteeman asked above

Hmm, not sure I understand what Brian asked. I think you refer to this?

I just wish there was a way that 3pd buttons had to "opt-in" to this changed (and improved) behaviour

Imho, there is no need to opt-in. The button plugins shouldn't care where they are shown. AS long as all plugins will display and work in the toolbar I think it's fine.

This obviously needs testing especially also with 3rd party plugins to see if they behave.

avatar brianteeman
brianteeman - comment - 20 Jun 2015

I would not be in favour of non-core plugins being moved automatically. eg the phocagallery, ozio or docman buttons

ONLY because we are changing the behaviour of something without the 3pd having control of it so all their documentation and tutorials etc will be broken

Example
http://www.mysysadmintips.com/tools/custom-text-button

If this applied to all buttons then we just broke this documentation without the 3pd knowing about it

I do WANT 3PD to have the opportunity to alter their code so they go inside Tiny but it should be their choice. They may decide for example that as it is only for Tiny and not for any other editor that they prefer to keep them where they are so that they are in the same place no matter what the editor is

What I am saying we should do something to our buttons so that you can target that when you look to insert them in the toolbar. Then a 3pd can choose to do the same if they want. Most will but I just dont think we should force it

avatar dgt41
dgt41 - comment - 20 Jun 2015

@brianteeman hehe you’re faster than me ????

avatar Bakual
Bakual - comment - 20 Jun 2015

Then a 3pd can choose to do the same if they want. Most will but I just dont think we should force it

I'd say most will not do it simply because they don't even know it's possible.

I'm not sure if we should not improve something (automatically for all buttons) just because some tutorials will become a bit outdated.
On the other hand I also don't know how hard it would be to make it optional. I have a feeling the code only gets much more complicate because of this. But that's something @dgt41 should say.

avatar brianteeman
brianteeman - comment - 20 Jun 2015

I dont think we should ever do something that fundamentally changes someone
elses code without their knowledge

On 20 June 2015 at 13:00, Thomas Hunziker notifications@github.com wrote:

Then a 3pd can choose to do the same if they want. Most will but I just
dont think we should force it

I'd say most will not do it simply because they don't even know it's
possible.

I'm not sure if we should not improve something (automatically for all
buttons) just because some tutorials will become a bit outdated.
On the other hand I also don't know how hard it would be to make it
optional. I have a feeling the code only gets much more complicate because
of this. But that's something @dgt41 https://github.com/dgt41 should
say.


Reply to this email directly or view it on GitHub
#7170 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

avatar dgt41
dgt41 - comment - 20 Jun 2015

@Bakual @brianteeman we could drop the native plugins for the joomla buttons from the other older PR set one more switch in tinymce and if the switch is set to yes get the 3PD buttons inside tinymce. But this will bring the decision to the user not the 3PD...

avatar Bakual
Bakual - comment - 20 Jun 2015

I dont think we should ever do something that fundamentally changes someone
elses code without their knowledge

Imho, it doesn't change code or function. Just the place where it's (visually) rendered. If it changes behaviour, then I agree.

avatar brianteeman
brianteeman - comment - 20 Jun 2015

Clearly I disagree

On 20 June 2015 at 13:45, Thomas Hunziker notifications@github.com wrote:

I dont think we should ever do something that fundamentally changes someone
elses code without their knowledge

Imho, it doesn't change code or function. Just the place where it's
(visually) rendered. If it changes behaviour, then I agree.


Reply to this email directly or view it on GitHub
#7170 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

avatar dgt41
dgt41 - comment - 20 Jun 2015

I also enabled 3PD ability to use their own icon

This needs a minor change to their inline css.
E.g.: For a button named mybutton change class from .btn-toolbar .icon-mybutton {} to .btn-toolbar .icon-mybutton, i.mce-ico.mce-i-mybutton {}

With custom icon
screenshot 2015-06-20 16 40 22

Without
screenshot 2015-06-20 16 43 06

So only thing to sort out I guess is @brianteeman ’s request?

avatar nikosdion
nikosdion - comment - 20 Jun 2015

@brianteeman Since I was the instigator of this PR and an affected 3PD developer I think I have to explain why this PR should be perfectly acceptable.

From a usability / documentation point of view

Right now editor-xtd buttons are rendered below the editor GUI in buttons, showing both an icon and a button title.

With the proposed PR the button row moves from below the editor into the editor. Only if you are using TinyMCE. Since the third party documentation talks about a new button being added to the row of editor buttons I don't see a huge issue here. Mind you, I am one of the affected 3PDs!

From a UX point of view

This is a godsend. Before the PR I would be damned if I knew that there were any editor buttons when I'm using my 13" MacBook Pro. It's not even a matter of scrolling further down. The editor buttons are TWO FULL WINDOW HEIGHTS below what I am seeing. Gargh! Are we still wondering why people think Joomla! is crap and not know how to use Read More links? Or why those who have found out where the heck those darned buttons are think that the article editor page was designed by a lobotomised monkey on acid? Or why those who know what they're doing use JCE even if they have no use of any of the extra features it packs? I'm sure you're aware of the issue. You are talking to end users as often as (or maybe more often than) I do.

The objective of this PR is to improve the user experience. To be perfectly honest, it even helps a seasoned user like me. I installed Akeeba Release System in the interest of testing this PR. Before the PR I had a hard time finding the ARS Item button even though I knew where it is supposed to be (having had a big meal and a couple of beers before didn't help). It made me think very hard and as a user I don't want to make me think. I want to focus on the content I want to write, not how the heck am I supposed to write it. After this PR the button was right in front of my eyes. I was already drunk and slow from eating a huge lunch and drinking a couple of beers and the button was still obviously there.

From a code point of view

We had two ways to go about it.

  1. Use the same plugin event (onDisplay), treating all plugins equally, no matter if they are core or 3PD. It also treats all the editors, core or third party, equally.

  2. Use a new plugin event (onDisplayToolbar) just for TinyMCE integration. This means a downgraded experience for 3PD plugins and code duplication. In fact, we'd be asking people to write duplicate code to support just one editor, the core TinyMCE one (which is not even the only core editor). How does that make sense?

Final word

The majority of Joomla! users are exposed to the very bad UX of our default TinyMCE editor. They have to scroll up and down two page heights just to insert a pagebreak, an image or a link to an article. Meanwhile WordPress offers them a much more reasonable editing environment. This leads to the perception that Joomla! is bad for editing content – or so I'm told by every single client who's switched to WordPress. At the same time those who know what they're doing don't use our default TinyMCE editor. Oh, sweet irony, they mostly use the other TinyMCE-based editor out there, JCE, because it has all useful buttons in the toolbar.

This PR does work (@test +1 by the way), it works well and it solves one of our major problems, the one which is in the core of what we are supposed to offer: content management. A CMS which sucks at content management isn't much of a Content Management System, is it now? So, can we finally stop whining about nonsense and get something positive done for a change? If we don't dare break bad patterns and UX we'll just keep on bleeding users. Let's just bloody fix it!

avatar brianteeman
brianteeman - comment - 20 Jun 2015

@nikosdion I love this PR and I have tested it and it works. I am just
unsure about changing the 3pd buttons without the 3pd opting in - that is
all.

Not even going to mention things that you yourself usually do when anyone
talks about changing the UI (remember the admin menu proposal) about it
breaking documentation and users screaming at you "where are my buttons">

All I am suggesting it that for a button to be included in the toolbar it
should have some code to enable that. Then a 3pd can choose to add that
code in an update or not.

On 20 June 2015 at 17:18, Nicholas K. Dionysopoulos <
notifications@github.com> wrote:

@brianteeman https://github.com/brianteeman Since I was the instigator
of this PR and an affected 3PD developer I think I have to explain why this
PR should be perfectly acceptable.
From a usability / documentation point of view

Right now editor-xtd buttons are rendered below the editor GUI in
buttons, showing both an icon and a button title.

With the proposed PR the button row moves from below the editor into the
editor. Only if you are using TinyMCE. Since the third party documentation
talks about a new button being added to the row of editor buttons I don't
see a huge issue here. Mind you, I am one of the affected 3PDs!
From a UX point of view

This is a godsend. Before the PR I would be damned if I knew that there
were any editor buttons when I'm using my 13" MacBook Pro. It's not even a
matter of scrolling further down. The editor buttons are TWO FULL WINDOW
HEIGHTS
below what I am seeing. Gargh! Are we still wondering why people
think Joomla! is crap and not know how to use Read More links? Or why those
who have found out where the heck those darned buttons are think that the
article editor page was designed by a lobotomised monkey on acid? Or why
those who know what they're doing use JCE even if they have no use of any
of the extra features it packs? I'm sure you're aware of the issue. You are
talking to end users as often as (or maybe more often than) I do.

The objective of this PR is to improve the user experience. To be
perfectly honest, it even helps a seasoned user like me. I installed Akeeba
Release System in the interest of testing this PR. Before the PR I had a
hard time finding the ARS Item button even though I knew where it is
supposed to be
(having had a big meal and a couple of beers before
didn't help). It made me think very hard and as a user I don't want to make
me think. I want to focus on the content I want to write, not how the heck
am I supposed to write it. After this PR the button was right in front of
my eyes. I was already drunk and slow from eating a huge lunch and drinking
a couple of beers and the button was still obviously there.
From a code point of view

We had two ways to go about it.

1.

Use the same plugin event (onDisplay), treating all plugins equally,
no matter if they are core or 3PD. It also treats all the editors, core or
third party, equally.
2.

Use a new plugin event (onDisplayToolbar) just for TinyMCE
integration. This means a downgraded experience for 3PD plugins and code
duplication. In fact, we'd be asking people to write duplicate code to
support just one editor, the core TinyMCE one (which is not even the
only core editor). How does that make sense?

Final word

The majority of Joomla! users are exposed to the very bad UX of our
default TinyMCE editor. They have to scroll up and down two page heights
just to insert a pagebreak, an image or a link to an article. Meanwhile
WordPress offers them a much more reasonable editing environment. This
leads to the perception that Joomla! is bad for editing content – or so I'm
told by every single client who's switched to WordPress. At the same time
those who know what they're doing don't use our default TinyMCE editor. Oh,
sweet irony, they mostly use the other TinyMCE-based editor out there, JCE,
because it has all useful buttons in the toolbar.

This PR does work (@test https://github.com/test +1 by the way), it
works well and it solves one of our major problems, the one which is in the
core of what we are supposed to offer: content management. A CMS which
sucks at content management isn't much of a Content Management
System, is it now? So, can we finally stop whining about nonsense and get
something positive done for a change? If we don't dare break bad patterns
and UX we'll just keep on bleeding users. Let's just bloody fix it!


Reply to this email directly or view it on GitHub
#7170 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

avatar dgt41
dgt41 - comment - 20 Jun 2015

@nikosdion @brianteeman just to mention that following the way of doing things in this pr we get one more thing that's high on my list: ability to remove mootools from the buttons (at least for tinymce €

avatar nikosdion
nikosdion - comment - 20 Jun 2015

Your proposal for menus allowed end users to REMOVE buttons completely. This PR merely puts buttons in a more sensible place, Brian. Comparing apples and oranges much?

avatar brianteeman
brianteeman - comment - 20 Jun 2015

No it was only to move them ;)

Anyway I expressed my view - its not my decision

On 20 June 2015 at 19:26, Nicholas K. Dionysopoulos <
notifications@github.com> wrote:

Your proposal for menus allowed end users to REMOVE buttons completely.
This PR merely puts buttons in a more sensible place, Brian. Comparing
apples and oranges much?


Reply to this email directly or view it on GitHub
#7170 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

avatar nikosdion
nikosdion - comment - 20 Jun 2015

Brian, I remember our conversation and the conclusion was that it would only make sense with an unremovable master menu reset button. The reason for that was that the suggestion would make the menu editable and the items removable, not just movable. I know very well why I object to something ;)

avatar smz smz - test_item - 20 Jun 2015 - Tested successfully
avatar smz
smz - comment - 20 Jun 2015

Tested OK! I like it!

Can't we use the same (nice!) icon you introduced for "Read More" also in the default button (I mean the one used when we are not using TinyMCE)?

P.S.: 3 tests, RTC?

avatar zero-24 zero-24 - alter_testresult - 21 Jun 2015 - JoshuaLewis: Not tested
avatar zero-24 zero-24 - alter_testresult - 21 Jun 2015 - brianteeman: Not tested
avatar zero-24
zero-24 - comment - 21 Jun 2015

P.S.: 3 tests, RTC?

Since the Code is changed after the testes. We need on more tester for RTC :smile:


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

avatar nikosdion
nikosdion - comment - 21 Jun 2015

@zero-24 I tested it myself after the last code changes. So now you do have 2 successful tests, mine and Sergio's. OK, more formally put:

@test Success. I've tested with my own editor-xtd plugin for Akeeba Release System. The PR works as expected, moving the editor-xtd buttons into the toolbar. I tested my button, which uses a modal to insert a link to a download item, successfully. It's +1 from me.

avatar zero-24 zero-24 - change - 21 Jun 2015
Status Pending Ready to Commit
avatar zero-24
zero-24 - comment - 21 Jun 2015

Sorry @nikosdion I did not see your test bevor. Than it is save to RTC :smile: Thanks.


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

avatar zero-24 zero-24 - alter_testresult - 21 Jun 2015 - nikosdion: Tested successfully
avatar dgt41
dgt41 - comment - 21 Jun 2015

Can somebody confirm that there are no bugs in a multiple instances environment (eg k2 with split intro-full area. Thanks

avatar infograf768
infograf768 - comment - 21 Jun 2015

@test
Works fine here (one instance).

avatar brianteeman
brianteeman - comment - 21 Jun 2015

Just for completeness I tested what would happen if you have too many buttons to display on single row and they wrap nicely
And tested the ordering of the buttons by changing their order in the plugin manager and that work as well


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

avatar brianteeman brianteeman - test_item - 21 Jun 2015 - Not tested
avatar brianteeman brianteeman - alter_testresult - 21 Jun 2015 - brianteeman: Tested successfully
avatar joomla-cms-bot joomla-cms-bot - change - 21 Jun 2015
Labels Added: ?
avatar dgt41
dgt41 - comment - 21 Jun 2015

#all I just realized that the public function onSave() is useless with tinyMCE 4, as the editor will automatically put the content on the text area field when the event form submit is called. So one more line of javascript less in the document!
I’ve tested it with one or multiple instances and it works fine.
Bad news we need people to verify that ???? Sorry about this, tinyMCE API sucks (or you can say @dgt41 PR’s sucks, same thing ????).
So edit an article, do some changes, save and close. Re open to verify that the content is there!

@smz I am afraid it is kinda hard to get that icon on the bootstrap buttons. Although we use the same font the icon, the page break doesn’t exist in the icoMoon font Joomla is using. The easiest way doing this is if somebody will ask icoMoon for a font update with that missing icon. I am not aware of any agreement between Joomla and icoMoon, so I guess someone more familiar with this subject will handle this easier.

avatar brianteeman
brianteeman - comment - 21 Jun 2015

Is there anyway to keep the icons that 3pd are using in their xtd buttons when they are added to the toolbar? Currently this PR is changing the 3pd provided code to use a different class for the icon and renames the icon resulting in the empty space

avatar dgt41
dgt41 - comment - 21 Jun 2015

@brianteeman In theory yes is doable. We need to beef up that foreach loop and do some pre_repace on the document->_styles. Actually this might be a great thing as 3PD will not have to do anything at all and we end up with nice icons immediately (no lag till someone catches up with this change). Let me try, that!

avatar brianteeman
brianteeman - comment - 21 Jun 2015

:clap: :clap:

avatar smz
smz - comment - 21 Jun 2015

@dgt41: more commits on their way? Should I wait re-testing?

avatar dgt41
dgt41 - comment - 21 Jun 2015

@brianteeman there we go!
@smz I think this complete, unless some bug comes up

#all Please verify that 3PD icons are correctly set, and that save still works as intended

avatar brianteeman
brianteeman - comment - 21 Jun 2015

Sorry - still not seeing 3pd images

[image: Inline images 1]

On 21 June 2015 at 12:43, Dimitris Grammatiko notifications@github.com
wrote:

@brianteeman https://github.com/brianteeman there we go!
@smz https://github.com/smz I think this complete, unless some bug
comes up

#all Please verify that 3PD icons are correctly set, and that save still
works as intended


Reply to this email directly or view it on GitHub
#7170 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

avatar dgt41
dgt41 - comment - 21 Jun 2015

@brianteeman can you send me those failing plugins at d.grammatiko [at] gmail ?
@brianteeman is it an image or and icon? For images this works for icons we might need something more. but I don’t have such a plugin to see the css code

avatar brianteeman
brianteeman - comment - 21 Jun 2015

three were free so you can download here

http://www.opensourcesolutions.es/en/ext/ozio-gallery.html
http://soft.dibuxo.com/SOFT/dxfa/dxfa_current.zip
https://joomla-extensions.kubik-rubik.de/downloads/sige-simple-image-gallery-extended

On 21 June 2015 at 12:50, Dimitris Grammatiko notifications@github.com
wrote:

@brianteeman https://github.com/brianteeman can you send me those
failing plugins at d.grammatiko [at] gmail


Reply to this email directly or view it on GitHub
#7170 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

avatar dgt41
dgt41 - comment - 21 Jun 2015

@brianteeman and now all should be fine! ????

avatar brianteeman
brianteeman - comment - 21 Jun 2015

Sorry thats worse. the 3pd buttons still dont have icons and the core image button has lost its icon

avatar dgt41
dgt41 - comment - 21 Jun 2015

@brianteeman is your localhost in a subfolder? Latest commit should fix that

avatar dgt41
dgt41 - comment - 21 Jun 2015

@smz did that and also minified the css to save some bytes over the wire

avatar smz
smz - comment - 21 Jun 2015

:+1: Seen that! Thanks! :smile:

avatar brianteeman
brianteeman - comment - 21 Jun 2015

Yes it was a subfolder

All good now

:clap: :clap:

On 21 June 2015 at 13:39, Sergio Manzi notifications@github.com wrote:

[image: :+1:] Seen that! Thanks! [image: :smile:]


Reply to this email directly or view it on GitHub
#7170 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

avatar smz
smz - comment - 21 Jun 2015

#test confirmed OK here too (but with no 3rd party buttons, and single instance)

avatar dgt41
dgt41 - comment - 21 Jun 2015

Just to document one limitation for the 3PD here (so I won’t forget it later on):
buttons cannot use one of the predefined classes in the tinybuttons.css AND ALSO DECLARE A BACKGROUND IMAGE. Either you use the predefined class or you name a custom not existing name!

In case you don’t follow this rule, this will happen:
screen shot 2015-06-21 at 3 45 11

avatar JoshuaLewis
JoshuaLewis - comment - 21 Jun 2015

The new version looks great and did not break my responsive template code. Icons render as expected:

editor-buttons-resized

However I really hate to note one issue that I really believe is worth re-testing for. The toggle editor button is gone with this PR. The toggle editor button is superior to the source code button based on how it displays the code in the editor rather than a small modal window. I would gladly be one of the folks to re-test bringing back the toggle button (Toggle editor) that could display as one of the inline buttons.

avatar dgt41
dgt41 - comment - 21 Jun 2015

@JoshuaLewis I don’t have a problem bringing that button back since (with combination to #7167) no mootools will get loaded. Also it is a really simple request

return 'if (tinyMCE.get("' . $editor . '").isHidden()) {tinyMCE.get("' . $editor . '").show()};';

$editor .= $this->_toogleButton($id);

return JLayoutHelper::render('joomla.tinymce.togglebutton', $name);

But I would like some more input about that. So let’s see what people think!

avatar JoshuaLewis
JoshuaLewis - comment - 21 Jun 2015

If we do end up removing it by default, is it possible to bring it back by putting it in the custom plugin/button field on a individual basis? Something like this (advanced tab of TinyMCE):

toggle-editor

This way we have the option to bring it back with the icon if we choose to do so.

avatar smz
smz - comment - 21 Jun 2015

For personal usage, I'm totally neutral on this: I don't use TinyMCE

On a general scale, as TinyMCE already has the source code button <> for opening a source editing modal, I think the Toggle editor button is redundant.

The solution proposed by @JoshuaLewis, if easily feasible, is elegant and could be a case for a community supported plugin: I'd love to have it in "Editor None" too, for switching to Codemirror. Switching between TinyMCE and Codemirror could be an option too...

avatar dgt41
dgt41 - comment - 21 Jun 2015

Actually ther is a plugin for tinymce to load code mirror for the source code https://github.com/PacificMorrowind/tinymce-codemirror. But that is far out of the scope of this PT

avatar smz
smz - comment - 21 Jun 2015

Interesting, yes! And out of scope, I agree! :smile:

Considering that there are alternative solutions (like the one you pointed at), my opinion (for what is worth...) is to leave this PR as it is now...

avatar JoshuaLewis
JoshuaLewis - comment - 22 Jun 2015

I'm willing to accept the toggle button not displaying by default. However I do feel that by not having a method to display it for those who want it we are actually removing a Joomla feature. I understand removing redundancy, however the source code button is not as good as the Toggle editor. The reason being that it's display is much smaller than the editor view, hence it's a bit more difficult to modify code or view it. The CodeMirror example looks interesting for sure, but it still suffers the same problem of having the code forced into a small modal box.

I would be perfectly happy if Toggle editor was hidden by default, but is accessible via TinyMCE's custom plugin/button backend field (as mentioned a few posts ago). This way the feature is not taken away, but is packed away for those who want it. There would be no features lost if we went this route and would remove the redundancy for the majority.

In other words, only displaying the Toggle editor to those who want it. :-) This is in my opinion the most ideal situation.

avatar Bakual
Bakual - comment - 22 Jun 2015

The toggle button has to stay. It's a different function from having the sourcecode modal.
It obviously also has to stay outside of the editor, as you otherwise don't have an option to bring the editor back :smile:
JCE has the same function as a simple link above the editor. However I would prefer to leave the button in the place it currently is.
Don't make it an option.

avatar dgt41
dgt41 - comment - 22 Jun 2015

@Bakual Can you review the changes?
@JoshuaLewis there you go: toggle button is back!
More tests needed ????

avatar smz
smz - comment - 22 Jun 2015

#test confirmed once more. This time with a 3rd party button too... :+1:

avatar JoshuaLewis JoshuaLewis - test_item - 22 Jun 2015 - Tested successfully
avatar JoshuaLewis
JoshuaLewis - comment - 22 Jun 2015

Tested again, looking good. :smile: In the future it would be neat to see the toggle editor button added to the toolbar. I guess the harder part is getting the other editor to display the button to toggle back.


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

avatar Bakual
Bakual - comment - 22 Jun 2015

In the future it would be neat to see the toggle editor button added to the toolbar. I guess the harder part is getting the other editor to display the button to toggle back.

That's why the button needs to stay outside the editor and its toolbar. Otherwise you can't get it back :smile:

avatar dgt41
dgt41 - comment - 22 Jun 2015

@Bakual Done!

avatar zero-24 zero-24 - change - 2 Jul 2015
Milestone Added:
avatar zero-24 zero-24 - change - 2 Jul 2015
Milestone Added:
avatar JoshuaLewis
JoshuaLewis - comment - 2 Jul 2015

The status of this says "Pending". I assume the only thing left is for it to be reviewed for it to be included in J 3.4.4?


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

avatar dgt41
dgt41 - comment - 2 Jul 2015

@JoshuaLewis most probably I have to rebase this again since there were some changes in tinymce.php (blame myself here!). So please wait till this is done and also 2 more tests will still be needed. I will ping you once I got this done!

avatar brianteeman
brianteeman - comment - 2 Jul 2015

As it will be a NEW feature then it will almost certainly be in 3.5.0
according to the rules of SemVer

On 2 July 2015 at 19:18, Dimitris Grammatiko notifications@github.com
wrote:

@JoshuaLewis https://github.com/JoshuaLewis most probably I have to
rebase this again since there were some changes in tinymce.php (blame
myself here!). So please wait till this is done and also 2 more tests will
still be needed. I will ping you once I got this done!


Reply to this email directly or view it on GitHub
#7170 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

avatar Bakual Bakual - change - 2 Jul 2015
Milestone Added:
avatar Bakual Bakual - change - 2 Jul 2015
Milestone Removed:
avatar Bakual
Bakual - comment - 2 Jul 2015

It's a bit of an edge case actually. You can look at it as a feature, but since it doesn't change any API it could also just be looked at as a visual design change, which would be fine in a patch.

However I would prefer when it goes into the next minor as it isn't really a fix for something.

avatar JoshuaLewis
JoshuaLewis - comment - 2 Jul 2015

It's a change in display behavior rather than a feature. I suppose this could also be considered a feature modification which is a little different from a added feature. I suppose it might slightly affect mods that people have. For example I use CSS to hide certain editor items, this PR changes up the id's, as a result changing what items are hidden. But from my point of view this is an easy fix on my part and was only an issue in the first place based on my own modification.

avatar dgt41
dgt41 - comment - 2 Jul 2015

So there will be a 3.4.4, I thought we were going directly to 3.5 ????

avatar Bakual
Bakual - comment - 2 Jul 2015

So there will be a 3.4.4, I thought we were going directly to 3.5

Since we never know what bugs arise and when 3.5 will be ready, the next release may be 3.4.4. The plan is however to go to 3.5.0 next.
But I'm no fortune teller and so it's best to keep the branches separated and maintain two milestones for now.

8dac7d9 2 Jul 2015 avatar dgt41 CS
3c82627 2 Jul 2015 avatar dgt41 CS
5f3a330 2 Jul 2015 avatar dgt41 CS
280f56e 3 Jul 2015 avatar dgt41 CS
avatar dgt41
dgt41 - comment - 3 Jul 2015

This is updated now, once again, to solve a merge conflict with the latest staging.
There is a false travis error but please ignore it (most tests pass successfully)
@JoshuaLewis @smz do you have some time to confirm that everything here still work correctly?
Thanks

avatar smz
smz - comment - 3 Jul 2015

Confirmed #test OK (on 3.4.4-dev)

All buttons in their place and functional.

avatar smz
smz - comment - 3 Jul 2015

Note: this Travis thing is starting to be a nuisance... :smirk:

avatar Bakual
Bakual - comment - 3 Jul 2015

Note: this Travis thing is starting to be a nuisance...

Indeed. If anyone has a clue where this is coming from, feel free to suggest an improvement.
Afaik it usually times out during composer update. Not sure what we can do there.

avatar dgt41
dgt41 - comment - 3 Jul 2015

I have no clue but travis.yml is throwing validation error:

screenshot 2015-07-03 14 41 29

Their documentation state:

notifications:
  webhooks:
    urls:
      - http://hooks.mydomain.com/travisci
      - http://hooks.mydomain.com/events
    on_success: [always|never|change] # default: always
    on_failure: [always|never|change] # default: always
    on_start: [always|never|change] # default: always

so on_start: false is not valid

avatar smz
smz - comment - 3 Jul 2015

@Bakual I don't have the slightest idea, but was wondering: if we can't solve this issue quickly, wouldn't it possible to have in issues.joomla.org a button (available only to the PR submitter) to relaunch the Travis job? Even better if this could be done on a "per PHP version" basis

avatar Bakual
Bakual - comment - 3 Jul 2015

@smz Maintainers can do that within Travis.
I think we can drop the whole notifications section in there. There is only the Gitter webhook and we don't use Gitter anymore (it was a short living test before Glip).

avatar smz
smz - comment - 3 Jul 2015

@smz Maintainers can do that within Travis.

Yes, I know, and in the vast majority of cases when asked they'll just do, but there have been cases where I asked and didn't got answer for a couple of days. I was shy to ask and re-ask again, so I resorted to eliminating a full-stop in a comment to reset things! :smile:

I also suppose this is a burden on maintainers...

avatar Bakual
Bakual - comment - 3 Jul 2015

It's not that much of an issue. When I see that Travis fails I check their result and ignore the error if it passed for 3/4 :smile:

avatar JoshuaLewis
JoshuaLewis - comment - 3 Jul 2015

@dgt41 After reverting and reapplying the patch it works as expected on both the frontend and backend. :-)

avatar zero-24 zero-24 - alter_testresult - 3 Jul 2015 - nikosdion: Not tested
avatar zero-24 zero-24 - alter_testresult - 3 Jul 2015 - brianteeman: Not tested
avatar zero-24
zero-24 - comment - 3 Jul 2015

RTC Thanks


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

avatar roland-d roland-d - reference | fc2fe38 - 17 Oct 15
avatar roland-d roland-d - close - 17 Oct 2015
avatar roland-d
roland-d - comment - 17 Oct 2015

Thanks everybody, merged into 3.5-dev with commit fc2fe3

avatar roland-d roland-d - change - 17 Oct 2015
Status Ready to Commit Closed
Closed_Date 0000-00-00 00:00:00 2015-10-17 10:47:15
Closed_By roland-d
avatar roland-d roland-d - close - 17 Oct 2015
avatar joomla-cms-bot joomla-cms-bot - close - 17 Oct 2015
avatar joomla-cms-bot joomla-cms-bot - change - 17 Oct 2015
Labels Removed: ?
avatar dgt41 dgt41 - head_ref_deleted - 4 Nov 2015
avatar nonumber
nonumber - comment - 11 Nov 2015

This PR breaks a lot of stuff, even core functionality.
See: #8378

Add a Comment

Login with GitHub to post a comment