Success

User tests: Successful: Unsuccessful:

avatar okonomiyaki3000
okonomiyaki3000
23 Aug 2013

Oh yeah.

avatar okonomiyaki3000 okonomiyaki3000 - open - 23 Aug 2013
avatar elinw
elinw - comment - 24 Aug 2013

w00t! Go @okonomiyaki3000 !

avatar okonomiyaki3000
okonomiyaki3000 - comment - 26 Sep 2013

Rebased with the latest master.

avatar phproberto
phproberto - comment - 5 Oct 2013

This is not mergeable. Can you rebase it again? Thanks!

avatar okonomiyaki3000
okonomiyaki3000 - comment - 7 Oct 2013

It's not just a matter of rebasing. #1875 breaks CodeMirror in a big way. We need another patch to fix the template manager before we can think about updating CodeMirror.

avatar okonomiyaki3000
okonomiyaki3000 - comment - 7 Oct 2013

Ah, I see that it has been mostly fixed. OK, rebasing.

avatar okonomiyaki3000
okonomiyaki3000 - comment - 7 Oct 2013

If this PR is accepted, #2159 is unnecessary. Those changes are also included here + a bunch of other good stuff.

avatar okonomiyaki3000
okonomiyaki3000 - comment - 11 Oct 2013

Rebased with the latest master.

avatar okonomiyaki3000
okonomiyaki3000 - comment - 27 Nov 2013

Looks like 3.20 is out (http://codemirror.net/) with a few nice improvements. I'll update again soon.

avatar betweenbrain
betweenbrain - comment - 27 Nov 2013

:+1:

avatar okonomiyaki3000
okonomiyaki3000 - comment - 28 Nov 2013

Oh, and the plugin params look a lot better now. They were a bit messy, especially in Joomla 3.2

avatar okonomiyaki3000 okonomiyaki3000 - change - 28 Nov 2013
Title
CodeMirror 3.16 - you're welcome.
CodeMirror 3.20 - you're welcome.
avatar okonomiyaki3000
okonomiyaki3000 - comment - 28 Nov 2013

Travis failed but I'm sure this error has nothing to do with my changes. What's up?

avatar Bakual
Bakual - comment - 28 Nov 2013

@okonomiyaki3000 This Travis error is unrelated to your change. Michael is aware of it but it is obviously a bit more complicate to fix as it depends on the enviroment somehow (as I understood it).

avatar okonomiyaki3000
okonomiyaki3000 - comment - 28 Nov 2013

I see. I'm running phpunit locally now on the untouched master branch and getting some failures. At least some of them seem to be related to windows line endings... If I can come up with some fixes I'll put them into a different PR.

avatar Fedik
Fedik - comment - 28 Nov 2013

@okonomiyaki3000 I noticed that in j3.2 the codemiror do not display the content when the editor in the hidden tab, until you click inside the editor...
you know whether they fixed it in new version?

avatar okonomiyaki3000
okonomiyaki3000 - comment - 28 Nov 2013

@Fedik I wasn't aware of that problem but I'll check it tomorrow and see it anything changed. If it's still an issue, I'll try to fix it.

avatar okonomiyaki3000
okonomiyaki3000 - comment - 29 Nov 2013

@Fedik I've confirmed this issue still exists. I suppose CodeMirror cannot render properly while hidden. There is a CodeMirror function refresh() which can be called and will fix the editor if it's in bad shape like that. So calling that function is probably the best solution but we must call it when the editor is visible. Luckily, Bootstrap's tabs fire an event when they are shown so we can use this event to trigger the CodeMirror refresh() function and all will be well. However, this means that the CodeMirror plugin needs to place some quite Bootstrap-specific code on the page. I'm not sure if that's such a hot idea. In theory, the editor could be used with a non-Bootstrap template.

If anyone has any thoughts about this, I'd love to hear them.

avatar Fedik
Fedik - comment - 29 Nov 2013

@okonomiyaki3000 thanks for checking.
in j3.2 in the my extension I use hack for call refresh() when Tab shown:

$(window).bind('load', function(){
        // fix codemiror display
        var codeMirors = $('.CodeMirror');
        $('a[href="#attrib-advanced"]').on('shown', function (e) {
                codeMirors.each(function(i, el){
                    el.CodeMirror.refresh();
                });
        });
 })

but how to make it globally for joomla!, I am not sure, because as you said: the editor could be used with a non-Bootstrap template

ok, I made new issue for this behavior, because it not really related to Codemiror upgrade, http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=32843&start=0

avatar okonomiyaki3000
okonomiyaki3000 - comment - 23 Jan 2014

Should I close this and open a new PR to staging instead of master?

avatar Bakual
Bakual - comment - 23 Jan 2014

You don't have to, we can handle it.
What this needs is testing.

avatar okonomiyaki3000
okonomiyaki3000 - comment - 24 Mar 2014

4.0 is out. The biggest new feature is multiple selections. There's some other cool stuff like SublimeText keybindings but I don't think that's activated by default. Maybe add some more options later.

avatar okonomiyaki3000
okonomiyaki3000 - comment - 18 Apr 2014

Rebased to fix merge conflicts.

avatar zero-24
zero-24 - comment - 18 Apr 2014

hi @okonomiyaki3000
:+1::+1::+1:
Thank you for your work!

The only thing i have see after install your branche Is the following:
Current behavior (3.2.4-dev)
templatemanager_3 2 4-dev
After the patch:
templatemanager_after_the_patch

So is it possible to make the CodeMirror a bit bigger at the template manager?

avatar okonomiyaki3000
okonomiyaki3000 - comment - 18 Apr 2014

@zero-24 please go to the plugin manager and have a look at the CodeMirror options. Be amazed.

avatar zero-24
zero-24 - comment - 18 Apr 2014

@okonomiyaki3000
:+1:
So is it possible to change the default font size to e.g. 13 or 14?

avatar okonomiyaki3000
okonomiyaki3000 - comment - 19 Apr 2014

Sure, I'm open to any suggestions for default settings. Line-height, font, size, theme, etc.

avatar zero-24
zero-24 - comment - 19 Apr 2014

@okonomiyaki3000
ok. I think the default size should be a bit higher (13 or 14). The others are ok.
The only question that we must answer is the default for the editor full sceen toggle. Bevor this patch it is (at the Template Manager) "Ctrl + Q". After this patch "F10"

I would not call it BC but it is a change and if it is possible to choose this as default why not do it? :+1:

avatar okonomiyaki3000
okonomiyaki3000 - comment - 21 Apr 2014

For full screen, I went with F-keys + zero or more modifiers because Ctrl+Anything just seemed like a bad idea. Ctrl-Q especially makes no sense to me as it's too similar to Cmd-Q which is always quit on all Mac applications. Since a tip is always displayed about this, I didn't think it would be a problem. If you think it will be an issue, I could add Ctrl-Q functionality while leaving the FKey as the 'official' way. Then, at some later date when Ctrl-Q has faded from memory, just remove it completely.

avatar okonomiyaki3000
okonomiyaki3000 - comment - 21 Apr 2014

There you go. Also #1832

avatar zero-24
zero-24 - comment - 25 Apr 2014

Yes good one (don't know the issue with MacOSX)
i have tested the 2.5.x one also successfull.
Thanks @okonomiyaki3000 !

avatar okonomiyaki3000
okonomiyaki3000 - comment - 25 Apr 2014

To be clear, there's no actual issue with OSX. CTRL-Q is not a conflict of any kind. But CMD-Q is the standard shortcut to quit used by basically every Mac application ever made. So then, CTRL-Q is not only non-intuitive, a simple mistake of using the wrong modifier could quit your browser and cause a loss of whatever you may have been working on. So I thought an FKey is a safer choice.

avatar okonomiyaki3000
okonomiyaki3000 - comment - 20 May 2014

Well... another update.

avatar okonomiyaki3000
okonomiyaki3000 - comment - 9 Jun 2014

Fira Mono is the coolest coding font.

avatar zero-24
zero-24 - comment - 9 Jun 2014

thanks @okonomiyaki3000 my tests with your branche still pass :+1:

avatar okonomiyaki3000
okonomiyaki3000 - comment - 26 Jun 2014

CodeMirror 4 is great, guys.

avatar roland-d
roland-d - comment - 26 Jun 2014

@test I have tried to apply this PR on the current staging branch but it fails:

D:\wamp\www\joomla-cms>curl https://github.com/joomla/joomla-cms/pull/1803.diff | git apply
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 2974k 100 2974k 0 0 1277k 0 0:00:02 0:00:02 --:--:-- 1350k
:37088: trailing whitespace.

:37192: trailing whitespace.
*
:37202: trailing whitespace.

  • Collection classes mixing in this trait provide a concrete :37207: trailing whitespace. * :37210: trailing whitespace. * error: patch failed: administrator/templates/isis/error.php:163 error: administrator/templates/isis/error.php: patch does not apply error: patch failed: administrator/templates/isis/index.php:146 error: administrator/templates/isis/index.php: patch does not apply

I find this odd because Travis passes. Not sure what to do next :-|

avatar okonomiyaki3000
okonomiyaki3000 - comment - 26 Jun 2014

The weirdest part is that this patch shouldn't event touch administrator/templates/isis/error.php or administrator/templates/isis/index.php

avatar okonomiyaki3000
okonomiyaki3000 - comment - 26 Jun 2014

oh....... looks like I rebased with staging instead of master. dammit.

avatar okonomiyaki3000
okonomiyaki3000 - comment - 26 Jun 2014

I see. The problem is that you're applying this patch to staging, you need to apply it to master. This is a very old PR from the time before staging. If need be, I can resubmit it to staging.

avatar brianteeman
brianteeman - comment - 26 Jun 2014

Closing in favour of #3833

avatar brianteeman brianteeman - change - 26 Jun 2014
Status New Closed
Closed_Date 0000-00-00 00:00:00 2014-06-26 09:15:12
avatar brianteeman brianteeman - close - 26 Jun 2014

Add a Comment

Login with GitHub to post a comment