@nikosdion Can you take a look on this one?

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

@nikosdion any better way to do this https://github.com/dgt41/joomla-cms/blob/__tiny_btns2/plugins/editors/tinymce/tinymce.php#L636-L643

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.

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

@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?

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

@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?

Sorry, no ideas :(

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.

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

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.

@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...

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.

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

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 😃.

@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.

@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!

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.

@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!

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.

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

@brianteeman hehe you’re faster than me 😃

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.

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

Confirmed #test OK (on 3.4.4-dev)

All buttons in their place and functional.

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

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.

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

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

@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

@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).

@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! 😄

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

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 😄

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

RTC Thanks

Thanks everybody, merged into 3.5-dev with commit fc2fe3

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