Ticket #4704 (closed enhancement: fixed)
[with patch, positive review] Use jquery to make the javascript code nicer
| Reported by: | jason | Owned by: | jason |
|---|---|---|---|
| Priority: | major | Milestone: | sage-3.3 |
| Component: | notebook | Keywords: | |
| Cc: | boothby, tclemans, robertwb | Work issues: | |
| Report Upstream: | Reviewers: | ||
| Authors: | Merged in: | ||
| Dependencies: | Stopgaps: |
Description
This ticket splits off part of #4267.
Attachments
-
jquery-javascript-cleanup.patch
(14.7 KB) -
added by jason 4 years ago.
Change History
comment:2 Changed 4 years ago by jason
- Owner changed from boothby to jason
- Status changed from new to assigned
comment:3 Changed 4 years ago by jason
- Summary changed from Use jquery to make the javascript code nicer to [with patch] Use jquery to make the javascript code nicer
comment:5 Changed 4 years ago by jason
- Summary changed from [with patch] Use jquery to make the javascript code nicer to [with patch, needs review] Use jquery to make the javascript code nicer
comment:6 Changed 4 years ago by was
This definitely needs to be commented on by Tom Boothby, since this patch simply deletes the entire optimized-for-us javascript AJAX "framework" he wrote, and replaces it by jQuery's. Is jQuery's actually better?
comment:8 Changed 4 years ago by boothby
- Cc tclemans, robertwb added
I'm not sure I see the point of this. I'm inclined to say, "If it ain't broke, don't fix it." One might accuse me of being biased... but I really don't think I am here -- I'd be happy to see my code go if it made the rest of the notebook code significantly better.
There's one benefit that I see:
'newcell=0' + '&id=' + id + '&input='+escape0('%__sage_interact__\n' + input))
becomes
{newcell: 0, id: id, input: '%__sage_interact__\n' + input}
and this is more readable (I'm hoping that the escape0 functionality is preserved). I'd like to see what was, bradshaw, and tclemans say about this. I'll test after finals are over.
comment:9 Changed 4 years ago by jason
I like the cleaner syntax. The other thing that may be nice about jquery is that it offloads maintaining to a third party that presumably has more time to focus on it. That may be a non-issue for our code right now, though.
comment:10 Changed 4 years ago by robertwb
If there isn't any behavior or performance degradation, I think cleaner syntax is worth having, especially if it makes it easier for more people to work on the notebook code. Also, assuming that jquery can do all we need it to, the more we can offload javascript ajax and browser compatibility code to a larger community the more time we can focus on writing interesting stuff.
comment:11 Changed 4 years ago by jason
boothby: could you test this patch now that finals are over?
comment:12 Changed 4 years ago by jason
boothby: I should mention that everything is automatically escaped, so the escape0 functionality is preserved, but is way more transparent to the user.
comment:13 Changed 4 years ago by TimothyClemans
- Type changed from defect to enhancement
Jason asked me to comment. I don't know Javascript, but I like the idea of outsourcing to an actively developed library.
comment:14 Changed 4 years ago by mabshoff
- Summary changed from [with patch, needs review] Use jquery to make the javascript code nicer to [with patch, positive review] Use jquery to make the javascript code nicer
- Milestone changed from sage-3.4.1 to sage-3.3
Positive review due to #4705. Jason commented that Tom gave his thumbs up to this patch.
Cheers,
Michael
comment:15 Changed 4 years ago by mabshoff
Merged jquery-javascript-cleanup.patch in Sage 3.3.alpha0
comment:16 Changed 4 years ago by mabshoff
- Status changed from assigned to closed
- Resolution set to fixed
Merged jquery-javascript-cleanup.patch in Sage 3.3.alpha0. Oops :)
comment:17 follow-up: ↓ 18 Changed 4 years ago by jason
When did I comment that Tom gave his thumbs up? I don't remember that.
comment:18 in reply to: ↑ 17 Changed 4 years ago by mabshoff
Replying to jason:
When did I comment that Tom gave his thumbs up? I don't remember that.
It come up in IRC, but I am no longer 100% sure it was you who told me or of it was Tom directly. Either way, the code is in and while it might have slipped in somewhat below standards SD 12 will see some pounding of the new code, so things should turn out ok :)
Cheers,
Michael
