? ? Pending

User tests: Successful: Unsuccessful:

avatar brianteeman
brianteeman
14 Jan 2020

PR for #16550

The tinymce save plugin doesnt work in joomla and instead of just doing nothing it discards edited article instead of saving it

This PR stops loading the save plugin so now if you press ctr-s you will just get any dialog your browser has set. Most importantly it does not result in you losing work

avatar brianteeman brianteeman - open - 14 Jan 2020
avatar brianteeman brianteeman - change - 14 Jan 2020
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 14 Jan 2020
Category Front End Plugins
avatar brianteeman
brianteeman - comment - 14 Jan 2020

@wilsonge I can port this super simple change to j4 if you dont want to have to deal with a merge

avatar jwaisner jwaisner - test_item - 14 Jan 2020 - Tested successfully
avatar jwaisner
jwaisner - comment - 14 Jan 2020

I have tested this item successfully on 5dd7cf7


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

avatar viocassel viocassel - test_item - 14 Jan 2020 - Tested successfully
avatar viocassel
viocassel - comment - 14 Jan 2020

I have tested this item successfully on 5dd7cf7


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

avatar Quy Quy - change - 14 Jan 2020
Status Pending Ready to Commit
avatar Quy
Quy - comment - 14 Jan 2020

RTC


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

avatar wilsonge
wilsonge - comment - 14 Jan 2020

@brianteeman at one point in the very first version of atum @dgrammatiko had the control + s actually saving the article. I'd rather get that functionality restored again than doing a direct port of removing the save plugin for j4 (obviously this is fine for j3)

avatar dgrammatiko
dgrammatiko - comment - 14 Jan 2020

@wilsonge there is a PR for that #24152 (although the js part should be part of the toolbar custom element)

avatar brianteeman
brianteeman - comment - 14 Jan 2020

even that pr requires this patch to work

I wanted to update that PR to use https://github.com/github/hotkey which is much more powerful but struggled with the "import"

avatar dgrammatiko
dgrammatiko - comment - 15 Jan 2020

I was reluctant on the es6 approach back in the days because what we have is a bastard between modern and legacy js. That project (the es6 transition) was never actually completed. In sort you cannot use import without a bundled like Webpack or rollup...

avatar brianteeman
brianteeman - comment - 15 Jan 2020

:( thats a shame because #24152 is restricted to using regular modifier keys and they always conflict

avatar dgrammatiko
dgrammatiko - comment - 15 Jan 2020

@brianteeman nope, that's fixable, eg:

	if (e.altKey && e.which == 83) {
                e.preventDefault();
 		var toolbar = document.getElementById("toolbar-apply");
 		toolbar.getElementsByClassName("button-apply")[0].click(); 
 	};

and change the event to fire onkeydown not onkeyup

PS. FWIW that code should be part of the toolbar CE js not another random js file

avatar brianteeman
brianteeman - comment - 15 Jan 2020

I dont want to do e.preventDefault(); as I consider it a bad idea to replace existing hotkeys - they might not be coming just from the browser (hint screenreaders) and thats why github, gmail etc all use a different modifier. (I can do it easily with mousetrap but you wouldnt let me)
and no it should not be part of the toolbar - the toolbar buttons are only a subset of where it could be used and we shouldnt limit ourselves

avatar dgrammatiko
dgrammatiko - comment - 15 Jan 2020

the toolbar buttons are only a subset of where it could be used and we shouldnt limit ourselves

Let me disagree here. You try to cover cases that nobody actually asked for and probably will be used by a minority of the users. You are making this thing one more plugin. Did anyone counted how many plugins J4 is introducing?
Just add the basic shortcuts that people expect to work there (ctr+S was the obvious one all the others are debatable).
Again adding features just to end up saying "hey Joomla provides also that" although it would be extremely hard to manage it and not to mention the customisation part. Why? Just why?
Simplicity is the ultimate design...

avatar brianteeman
brianteeman - comment - 15 Jan 2020

rofl says the man who introduced hundreds of javascript files

avatar dgrammatiko
dgrammatiko - comment - 15 Jan 2020

rofl says the man who introduced hundreds of javascript files

You do understand that you comment is irrelevant. Did I actually brought to the project a deprecated/outdated script like the one from eBay or the mousetrap that you mentioned above? No. So please...

avatar C-Lodder
C-Lodder - comment - 15 Jan 2020

Is it just me or does this feature keep getting broken in J4?

avatar wilsonge
wilsonge - comment - 15 Jan 2020

It only got broken once. and then never fixed. so not keep getting broken no :P

avatar brianteeman
brianteeman - comment - 15 Jan 2020

It never worked in joomla 3

avatar brianteeman
brianteeman - comment - 15 Jan 2020

The original report was in June 2017

avatar dgrammatiko
dgrammatiko - comment - 15 Jan 2020

That plugin rightfully needs to be removed:
Screenshot 2020-01-15 at 16 04 42

This PR has nothing to do with the ctrl+S combo @wilsonge and @C-Lodder talking about. That is another conversation, what the project should offer by default and why. Also the implementation needs to be tightly connected to the toolbar as the events actually trigger those buttons...

My 2c

avatar brianteeman
brianteeman - comment - 15 Jan 2020

Just merge this and get on with it

avatar Quy Quy - change - 15 Jan 2020
Labels Added: ? ?
avatar brianteeman
brianteeman - comment - 21 Jan 2020

Having discussed the use of custom keys and/or modifying keys used by the browser with Bruce regarding accessibility it was deemed to be a bad idea to change the expected behaviour of a browser

avatar wilsonge wilsonge - change - 21 Jan 2020
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2020-01-21 12:13:31
Closed_By wilsonge
avatar wilsonge
wilsonge - comment - 21 Jan 2020

Thanks!

Add a Comment

Login with GitHub to post a comment