User tests: Successful: Unsuccessful:
Oh yeah.
w00t! Go @okonomiyaki3000 !
Rebased with the latest master.
This is not mergeable. Can you rebase it again? Thanks!
Ah, I see that it has been mostly fixed. OK, rebasing.
Rebased with the latest master.
Looks like 3.20 is out (http://codemirror.net/) with a few nice improvements. I'll update again soon.
Oh, and the plugin params look a lot better now. They were a bit messy, especially in Joomla 3.2
Title |
|
Travis failed but I'm sure this error has nothing to do with my changes. What's up?
@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).
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.
@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?
@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.
@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
Should I close this and open a new PR to staging instead of master?
You don't have to, we can handle it.
What this needs is testing.
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.
Rebased to fix merge conflicts.
hi @okonomiyaki3000
Thank you for your work!
The only thing i have see after install your branche Is the following:
Current behavior (3.2.4-dev)
After the patch:
So is it possible to make the CodeMirror a bit bigger at the template manager?
@okonomiyaki3000
So is it possible to change the default font size to e.g. 13 or 14?
Sure, I'm open to any suggestions for default settings. Line-height, font, size, theme, etc.
@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?
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.
Yes good one (don't know the issue with MacOSX)
i have tested the 2.5.x one also successfull.
Thanks @okonomiyaki3000 !
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.
Well... another update.
thanks @okonomiyaki3000 my tests with your branche still pass
CodeMirror 4 is great, guys.
@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.
I find this odd because Travis passes. Not sure what to do next :-|
The weirdest part is that this patch shouldn't event touch administrator/templates/isis/error.php or administrator/templates/isis/index.php
oh....... looks like I rebased with staging instead of master. dammit.
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.
Status | New | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2014-06-26 09:15:12 |
Tracker: http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=31822&start=0