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

[5.0] [new feature] TinyMce sample image plugin #36968

Closed
wants to merge 8 commits into from

Conversation

brianteeman
Copy link
Contributor

@brianteeman brianteeman commented Feb 8, 2022

This is a toolbar icon for the tinymce editor that will let you quickly generate a placeholder image in your content.

To test

  1. Apply pr
  2. rebuild the js with npm run build:js
  3. Go to the tinymce plugin and drag the new icon to your toolbar
  4. Go to any article and try it you

Plugin

tiny

Demo

tiny2

This is a toolbar icon for the tinymce editor that will let you quickly generate a placeholder image in your content.

To test
1. Apply pr
2. rebuild the js with `npm run build:js`
3. Go to the tinymce plugin and drag the new icon to your toolbar
4. Go to any article and try it you
@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.2-dev labels Feb 8, 2022
@joomla-cms-bot joomla-cms-bot added the Language Change This is for Translators label Feb 8, 2022
@laoneo
Copy link
Member

laoneo commented Feb 9, 2022

Why do you need that? I don't think this needs to come in core. Write a plugin and load it through the plugin feature if you need it.

@brianteeman
Copy link
Contributor Author

It speeds up development and makes life easier to build a site. Feedback was very positive when I showed the concept on FB. We have to start treating tinymce as a first class citizen. Instead we push people to use an editor that is old and outdate but has the extras people need.

@laoneo
Copy link
Member

laoneo commented Feb 10, 2022

Don't agree here. The core should act as an example and doesn't need to compete out 3rd party extensions. Bloating the core with such useless features which will not bring any benefit to the users is a wrong direction. Instead of, you should tell the users how extensible the core editor is and make an example of this use case either in the official docs or in a magazine post.

@brianteeman
Copy link
Contributor Author

I couldnt disagree more

@Fedik
Copy link
Member

Fedik commented Feb 14, 2022

I think the idea is good, can be useful for authors, it allow to format the article and replace images to final ones later.

@HLeithner
Copy link
Member

I don't think it should be in core if it's possible to do it with a Plugin. A 3rd Party Plugin could also use 3rd Party image services.

@Fedik
Copy link
Member

Fedik commented Feb 14, 2022

I know, but that something that Writers would like to see in core.

@brianteeman
Copy link
Contributor Author

@HLeithner you are missing the point. Core must be usable and useful. It doesnt matter how fast the code is or how modern the code is. What matters is that users can use it. Why should they have to install a plugin for something that is 40 lines of code. You can make the argument about making it a 3rd party extension with every new feature added to core in J4. Workflows -> almost unusable in its current form without extra code. Schedule -> completely unusable without extra code. Media Manager - the least said abut the better.
PS it is not possible to do this with an installable plugin that is fully integrated into the editor

@brianteeman
Copy link
Contributor Author

Forgot to add. I actually did some (rudimentary) user research before even starting this. When was the last time anyone did that

@laoneo
Copy link
Member

laoneo commented Feb 14, 2022

TinyMCE has a feature where you can load TinyMCE plugins by adding it to the options. We do not talk here about a Joomla plugin.

@brianteeman
Copy link
Contributor Author

You can make this argument about everything. Its a stupid argument. We must make joomla usable.

@brianteeman
Copy link
Contributor Author

TinyMCE has a feature where you can load TinyMCE plugins by adding it to the options. We do not talk here about a Joomla plugin.

Show me how a joomla user can install that plugin. There is no functionality for that.

@laoneo
Copy link
Member

laoneo commented Feb 14, 2022

The part on the bottom:

image

I'm using it on my production sites with the autolink plugin.

@Fedik
Copy link
Member

Fedik commented Feb 14, 2022

@laoneo please do not forget that Joe Average does not have a skills to code it (and this plugin exists nowhere) 😉
He want to write a text.

@brianteeman
Copy link
Contributor Author

@laoneo I am well aware of that option. Still doesn't answer the fact that there is no way on joomla to install that plugin.

There has been more lines of comment than there has been code

@laoneo
Copy link
Member

laoneo commented Feb 14, 2022

Instead of, you should tell the users how extensible the core editor is and make an example of this use case either in the official docs or in a magazine post.

Again, publish your sample lines and tell the world how extensible it is.

@laoneo I am well aware of that option. Still doesn't answer the fact that there is no way on joomla to install that plugin.

Uploading it via FTP or the template editor is enough. There is really nothing to install here. Again, this is a great showcase how to use the plugin feature of TinyMCE.

@laoneo
Copy link
Member

laoneo commented Feb 14, 2022

@laoneo please do not forget that Joe Average does not have a skills to code it (and this plugin exists nowhere) wink He want to write a text.

Copy paste is enough for the few people who are using it.

@brianteeman
Copy link
Contributor Author

I wonder how you would have reacted if I had told you to just write a blog post about custom fields. No need for that to be in the core. Just copy paste code, use ftp etc.

@laoneo
Copy link
Member

laoneo commented Feb 14, 2022

I wonder how you would have reacted if I had told you to just write a blog post about custom fields. No need for that to be in the core. Just copy paste code, use ftp etc.

It's ok, no need to shift away. I shared my opinion. Do whatever you guys want. It really doesn't matter if this feature is in or not at the end of the day.

@richard67 richard67 added the PBF Pizza, Bugs and Fun label Apr 22, 2022
@RickR2H
Copy link
Member

RickR2H commented Apr 22, 2022

I have tested this item ✅ successfully on fa08447


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

@brianteeman
Copy link
Contributor Author

closed due to lack of interest from release lead

@HLeithner
Copy link
Member

This pull request has been automatically rebased to 5.0-dev.

@fancyFranci
Copy link
Contributor

I have tested this item 🔴 unsuccessfully on fa08447

First the icon did not load in the Tinymce plugin view, just the placeholder text. After reloading the page the icon appeared as expected. I can drag it to my toolbar and save it. But then I can not open the article any more. A soon as I revert the patch its working again. I think placeholder images are really useful, would be great if you can fix that.

"An error has occurred.
0 Call to a member function getIdentity() on null "


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

@brianteeman
Copy link
Contributor Author

brianteeman commented Aug 26, 2023

I can't replicate your issue got confused with another pr

@HLeithner
Copy link
Member

I have tested this item 🔴 unsuccessfully on fa08447First the icon did not load in the Tinymce plugin view, just the placeholder text. After reloading the page the icon appeared as expected. I can drag it to my toolbar and save it. But then I can not open the article any more. A soon as I revert the patch its working again. I think placeholder images are really useful, would be great if you can fix that.

"An error has occurred. 0 Call to a member function getIdentity() on null "

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

a trace log would be helpful

@fancyFranci
Copy link
Contributor

a trace log would be helpful

You're right.

grafik

@HLeithner
Copy link
Member

@Fedik could this be of the change from pseudo event buttons to real events?
@laoneo or did you missed something for identity here?

@fancyFranci
Copy link
Contributor

@HLeithner I really dont know why, but after I tested #36975 with the same result, I went back to here, installed again and the error is gone now... Inserting sample images works as expected.

@fancyFranci
Copy link
Contributor

I have tested this item ✅ successfully on e2ada52

I was able to add sample images into my article


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

@fancyFranci
Copy link
Contributor

The error comes back when I remove the other new plugin (sample text) from the tinymce toolbar. After I tested it and left it there, the sample image worked.

@Fedik
Copy link
Member

Fedik commented Aug 26, 2023

could this be of the change from pseudo event buttons to real events?

Nope, this can happen when $application not injected in to plugin while bootPlugin,

$user = $this->getApplication()->getIdentity();

It nothing with events.
But it should be injected already

$plugin->setApplication(Factory::getApplication());

@Fedik
Copy link
Member

Fedik commented Aug 26, 2023

@fancyFranci did you tested iit for joomla 5 or 4? because title say 4,but the target is 5 :)

@Fedik Fedik changed the title [4.2] [new feature] TinyMce sample image plugin [5.0] [new feature] TinyMce sample image plugin Aug 26, 2023
@N6REJ
Copy link
Contributor

N6REJ commented Aug 26, 2023

I have tested this item 🔴 unsuccessfully on e2ada52 using J5.0-dev

applying the patch causes tinymce to no longer appear


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

pre-patch
image

Post patch
image

@brianteeman
Copy link
Contributor Author

a trace log would be helpful

@brianteeman
Copy link
Contributor Author

The PR was created for Joomla 4.2 using tinymce 5. @HLeithner changed this to be for 5.0 but as that uses tinymce 6 and a reworked plugin tbh I wouldnt expect this to work at all.

@N6REJ
Copy link
Contributor

N6REJ commented Aug 26, 2023

The PR was created for Joomla 4.2 using tinymce 5. @HLeithner changed this to be for 5.0 but as that uses tinymce 6 and a reworked plugin tbh I wouldnt expect this to work at all.

ok, so this fails j5.

@HLeithner
Copy link
Member

same as the other pr, why not using the template system for this?

@HLeithner HLeithner closed this Sep 3, 2023
@brianteeman brianteeman deleted the tinyimage branch September 3, 2023 08:04
@brianteeman
Copy link
Contributor Author

why not using the template system for this?

because the template system is not easily portable and is template dependent

@HLeithner
Copy link
Member

didn't we had a standard set/location in joomla too? or was this located only in Cassiopiea? Anyway tinymce deprecated the "simple" template system for there paid advanced template system. So we forked it and maybe @Fedik or @dgrammatiko or @laoneo has a nice idea for a better template system for such cases.

@Fedik
Copy link
Member

Fedik commented Sep 3, 2023

Following code is looking for templates

$filepaths = Folder::exists(JPATH_ROOT . '/templates/' . $template)
? Folder::files(JPATH_ROOT . '/templates/' . $template, '\.(html|txt)$', false, true)
: [];

Not sure what exactly, I think @dgrammatiko know better.
It is part of #40626

@Fedik
Copy link
Member

Fedik commented Sep 3, 2023

it maybe a good idea to have some default templates under /media/plg_editors_tinymce/templates/

@dgrammatiko
Copy link
Contributor

Not sure what exactly, I think @dgrammatiko know better.

That ajax fn just replaced the code that resolved the templates on startup (now it's per use case)

it maybe a good idea to have some default templates under /media/plg_editors_tinymce/templates/

It's very easy to add one more lookup path in the code you linked above #36968 (comment)

@brianteeman
Copy link
Contributor Author

do it without me. wasted enough time on making a good editor for joomla already without an 18 month delay to everything

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Language Change This is for Translators NPM Resource Changed This Pull Request can't be tested by Patchtester PBF Pizza, Bugs and Fun
Projects
None yet
Development

Successfully merging this pull request may close these issues.