refactor: [AXM-361] refactor JS bridge for IOS and Android #2580
Conversation
| } | ||
|
|
||
| const originalAjax = $.ajax; | ||
| $.ajax = function (options) { |
There was a problem hiding this comment.
Hmmm, I saw the ajax patch, and thought maybe in the final version we will attach event directly to the Submit event or element that submits it, but it's ok for us now
lms/static/js/courseware/bridge.js
Outdated
| } | ||
|
|
||
| if (window.AndroidBridge) { | ||
| window.addEventListener("messageFromAndroid", function (event) { |
There was a problem hiding this comment.
Ok, those events are not according to specification
the new event to listeners for markProblemCompletedIOS and markProblemCompletedAndroid events are registered for the window
lms/static/js/courseware/bridge.js
Outdated
| function markProblemCompleted(message) { | ||
| const data = JSON.parse(message).data; | ||
|
|
||
| const prob = $(".xblock-student_view"); |
There was a problem hiding this comment.
Please be more discriptive, instead of prob, please name it problemContainer
| markProblemCompleted(message); | ||
| } | ||
|
|
||
| function markProblemCompleted(message) { |
There was a problem hiding this comment.
Please add JSdoc to every function in this file
Like this -
lms/static/js/courseware/bridge.js
Outdated
| window?.AndroidBridge?.postMessage(message); | ||
| } | ||
|
|
||
| function markProblemCompletedIOS(message) { |
There was a problem hiding this comment.
There's no need to do these 2 proxy calls if there are no differences in IOS and Android calls
There was a problem hiding this comment.
Yes, there are none at the moment (although we haven't had any feedback from IOS yet), but if in the future there is a need for this, it will allow us to make changes to only one function, without having to change the names on the mobile side.
There was a problem hiding this comment.
I believe the name of the function doesn't mean much we should relly on JS Event names, am I correct?
window.addEventListener("messageFromIOS"
NiedielnitsevIvan
force-pushed
the
NiedielnitsevIvan/AXM-361/feature/Implement-JS-bridge-on-the-server-side-for-Problem-XBlock
branch
from
3801f56 to
01ff507
Compare
lms/static/js/courseware/bridge.js
Outdated
| */ | ||
| const originalAjax = $.ajax; | ||
| $.ajax = function (options) { | ||
| if (options.url && options.url.endsWith("problem_check")) { |
There was a problem hiding this comment.
I've thoght about it only now, but let's change the endsWith to check more specific url entry
handler/xmodule_handler/problem_check
lms/static/js/courseware/bridge.js
Outdated
|
|
||
| data.split("&").forEach(function (item) { | ||
| const [inputId, answer] = item.split('=', 2); | ||
| problemContainer.find('input[id$="' + answer + '"], input[id$="' + inputId + '"]').each(function () { |
There was a problem hiding this comment.
Can we use template string here?
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Template_literals
There was a problem hiding this comment.
I'll give it a try, because it was a solution for js when it was in an html file and had to build it through mako.
NiedielnitsevIvan
force-pushed
the
NiedielnitsevIvan/AXM-361/feature/Implement-JS-bridge-on-the-server-side-for-Problem-XBlock
branch
from
01ff507 to
af88007
Compare
NiedielnitsevIvan
force-pushed
the
NiedielnitsevIvan/AXM-361/feature/Implement-JS-bridge-on-the-server-side-for-Problem-XBlock
branch
from
af88007 to
0202935
Compare
AXM-361 Implement JS bridge on the server side for Problem XBlock