#10664 closed New Feature (fixed)
MathJax widget
Reported by: | Piotr Jasiun | Owned by: | Piotr Jasiun |
---|---|---|---|
Priority: | Normal | Milestone: | CKEditor 4.3 Beta |
Component: | General | Version: | |
Keywords: | Cc: | wim.leers@… |
Description
Widget allows you insert and edit mathematical equations in TeX or MathML and display them with MathJax library.
Change History (18)
comment:1 Changed 11 years ago by
Owner: | set to Piotr Jasiun |
---|---|
Status: | new → assigned |
comment:2 Changed 11 years ago by
Cc: | wim.leers@… added |
---|
comment:4 Changed 11 years ago by
Replying to Reinmar:
I made a short review and I have some comments.
- data.math - I'm not sure if this is a good name. It contains equation/formula's source and "math" seems to be meaningless.
I was looking for a good name for this. But it could be equation, inequality, expression, variable... I checked how MathJax call it and they use 'mathematics' ('open source JavaScript display engine for mathematics that works in all browsers.'). But if you have any better idea I will be be grateful.
data.math = el.children[ 0 ].value;
such careless traversing structure coming from data is unsafe. In this case it only causes displaying default equation, but some check what's there would be better and if span's content is corrupted we should rather not upcast it.
Fixed.
- getAbsolutePath - I've seen similar hack somewhere else. It'd be great to not have it inside widget.
We decided to remove it.
- The code lacks comments.
I know. I will work on it.
comment:5 Changed 11 years ago by
Status: | assigned → review |
---|
Widget is ready to review. I will keep working on it but my changes shouldn't be critical.
My TODO list:
- more text in sample,
- comments & documentation,
- fix same origin policy issue,
- tests for: inline editor
- tests for: undo/redo
- tests for: absolute path and preview plugin
- and some minior tests.
comment:6 Changed 11 years ago by
Status: | review → review_failed |
---|
I've just pushed small changes. Still we have issues.
With Firefox:
- Clean the document.
- Click the Math button.
- Click ok to insert the default math.
- Check the source.
You will have this:
<p><span class="math-tex">\(x = {-b \pm \sqrt{b^2-4ac} \over 2a}\)</span><img src="data:image/gif;base64,R0lGODlhAQABAPABAP///wAAACH5BAEKAAAALAAAAAABAAEAAAICRAEAOw%3D%3D" /><span style="background:url("http://ckeditor.dev/plugins/widget/images/drag.png") repeat scroll 0% 0% rgba(220, 220, 220, 0.5); left:0px; top:-14px"><img draggable="true" src="data:image/gif;base64,R0lGODlhAQABAPABAP///wAAACH5BAEKAAAALAAAAAABAAEAAAICRAEAOw%3D%3D" style="height:14px; width:14px" /></span></p>
I'll keep the review going here meanwhile.
comment:7 Changed 11 years ago by
I pushed a fix to t/10664 for the issue mentioned in comment:6. I added some points to the https://docs.google.com/document/d/1U4oUCuk4gLuXUzx5BFyN1VlDEmVEggK-ovapSqpXkBQ/edit?usp=sharing
comment:8 follow-up: 9 Changed 11 years ago by
Very nice :)
Feedback:
- The dialog title says "Mathematics in TeX". The same text is repeated just above the <textarea>.
- The dialog should probably link to TeX documentation, to educate the user about the syntax?
- Why not get rid of the dialog all together? As soon as the cursor reaches a MathJax widget, the formula itself could be displayed *right there*, and the user would be able to edit it. No need for a dialog. Direct manipulation. Of course, that would mean the user won't get a preview. But a preview is only really necessary for more complex formulas. So why not allow double-clicking the widget to get a dialog, otherwise in-place editing? If you don't do that, there's little reason to use a Widget for this, isn't there? Widgets are about removing the need for dialogs (out-of-place editing) where possible, aren't they.
comment:9 Changed 11 years ago by
Thanks for your feedback :)
- The dialog title says "Mathematics in TeX". The same text is repeated just above the
<textarea>
.
I also don't like this. Do you have any better idea?
- The dialog should probably link to TeX documentation, to educate the user about the syntax?
Good idea. I will add this.
- Why not get rid of the dialog all together? As soon as the cursor reaches a MathJax widget, the formula itself could be displayed *right there*, and the user would be able to edit it. No need for a dialog. Direct manipulation. Of course, that would mean the user won't get a preview. But a preview is only really necessary for more complex formulas. So why not allow double-clicking the widget to get a dialog, otherwise in-place editing?
If you don't do that, there's little reason to use a Widget for this, isn't there? Widgets are about removing the need for dialogs (out-of-place editing) where possible, aren't they.
The idea of this widget was to show power of widget system, without plenty of code. In-place-editing (without dialog) would be a great feature, but we can add it in the future. Let keep it simple at the begging.
comment:10 Changed 11 years ago by
Why not get rid of the dialog all together? As soon as the cursor reaches a MathJax widget, the formula itself could be displayed *right there*, and the user would be able to edit it. No need for a dialog. Direct manipulation. Of course, that would mean the user won't get a preview. But a preview is only really necessary for more complex formulas. So why not allow double-clicking the widget to get a dialog, otherwise in-place editing? If you don't do that, there's little reason to use a Widget for this, isn't there? Widgets are about removing the need for dialogs (out-of-place editing) where possible, aren't they.
In this case widgets system is not used to enable in-place editing, but to solve a little bit different problem. MathJax library is quite destructive and demanding - it does A LOT of operations on DOM and therefore it would simply destroy editor. Thanks to widgets we have control over life cycle of an iframe which we used to separate MathJax context from editor context. This wasn't possible before.
So we are now able to render equations directly in the editor, which is already a great improvement. They can also be edited and during edition there's a live preview. Yes, this doesn't happen directly in editor's area, but it's consistent with other entities which are editable using dialog.
I also don't know where else could we display an input for a TeX format. In a small popup over editable? That would be something completely new and perhaps this is a way for the future, but now we don't have such system with a11y support and so on, that's why we chose the dialogs system.
comment:11 follow-up: 12 Changed 11 years ago by
I also don't like this. Do you have any better idea?
Only keep the one in the dialog title.
Let keep it simple at the begging.
I agree with the principle. KISS FTW. But at the same time, the demos have to be convincing. Wowing, preferably.
I also don't know where else could we display an input for a TeX format.
Replace the <span class="mathjax">MATHEMATICAL FORMULA HERE</span>
with <span class="mathjax-editing">MATHEMATICAL FORMULA HERE</span>, so that MathJax doesn't do its magic, and the user can edit the formula right there. Upon pressing the return key, or giving focus to something else, transform to
<span class="mathjax">…` again, so that MathJax once again performs its magic.
comment:12 Changed 11 years ago by
Replying to Wim Leers:
I also don't like this. Do you have any better idea?
Only keep the one in the dialog title.
We cannot do this because of accessibility. But I changed the second header.
Let keep it simple at the begging.
I agree with the principle. KISS FTW. But at the same time, the demos have to be convincing. Wowing, preferably.
I also don't know where else could we display an input for a TeX format.
Replace the
<span class="mathjax">MATHEMATICAL FORMULA HERE</span>
with <span class="mathjax-editing">MATHEMATICAL FORMULA HERE</span>, so that MathJax doesn't do its magic, and the user can edit the formula right there. Upon pressing the return key, or giving focus to something else, transform to
<span class="mathjax">…` again, so that MathJax once again performs its magic.
comment:13 Changed 11 years ago by
Status: | review_failed → review |
---|
Piotrek fixes problem which was the reason of R-. I also added some text to sample, documentation and code comments. I made some changes in dialog as well.
On my TODO list is (still):
- adding some more tests (I will work on them during the review),
- fix same origin policy (but it will be done after beta release).
comment:14 Changed 11 years ago by
We cannot do this because of accessibility.
Just make it only visible for screenreaders? :)
comment:15 Changed 11 years ago by
More comments:
- When a DOM subtree in which widget is attached changes, widget is unloaded. The preview disappears and widget is not responding on doubleclicks. To reproduce - select few paragraphs and apply bold/list/whatever.
- On Chrome very often scrolling by mouse the browser's viewport stops working :| It can be unblocked only by dragging the scrollbar by mouse - then it works. Not sure if we can do anything about this, but it's weird ;/
- On Chrome and IE I wasn't able to undo change done through a dialog to the widgets which are alone in their paragraphs. This is most likely caused by #10812, because there's an error thrown from the selection.js. And it works fine on FF (because there's a bogus br after widget, so it doesn't fall into #10812 case).
None of these is a beta blocker although 1. is very uncool.
comment:16 Changed 11 years ago by
Replying to Reinmar:
None of these is a beta blocker although 1. is very uncool.
I know about this and agree that this is very uncool. I talked with Fred about this and we decided to fix it after beta.
comment:17 Changed 11 years ago by
Status: | review → review_passed |
---|
Please report tickets for every still opened issue which we mentioned above and assign them to 4.3.
comment:18 Changed 11 years ago by
Resolution: | → fixed |
---|---|
Status: | review_passed → closed |
- git:2ec32e7
- tests:2ec32e7
I made a short review and I have some comments.
data.math = el.children[ 0 ].value;
such careless traversing structure coming from data is unsafe. In this case it only causes displaying default equation, but some check what's there would be better and if span's content is corrupted we should rather not upcast it.