Fix part of #19435: Learner group editor page migration #19761
Conversation
|
Hi @AFZL210 please assign the required reviewer(s) for this PR. Thanks! |
|
@DubeySandeep addressed all comments, but this comment though it is handled 100% as BLOG_AUTHOR_PROFILE_PAGE also has an extra param but I didn't find the exact reason why it's working. |
|
@kevintab95 Can you please review this PR? |
| <script> | ||
| // Use the normal default browser eventlistener without passive | ||
| // set to false. | ||
| // TODO(#10749): Remove workaround for passive event listeners when | ||
| // jQuery supports it. | ||
| EventTarget.prototype.addEventListener = ( | ||
| EventTarget.prototype.addEventListener._original); | ||
| </script> |
There was a problem hiding this comment.
Should we add this in the migrated HTML? Like in
There was a problem hiding this comment.
although I don't have much idea about this, looking at the issue and the explanation for this in the previous PR, I believe we should add this.
|
Unassigning @kevintab95 since they have already approved the PR. |
|
Hi @AFZL210, this PR is ready to be merged. Please address any remaining comments prior to merging, and feel free to ask someone to merge your PR once the CI checks pass and you're happy with it. Thanks! |
Overview
Learner Group Editorpage from Webpack/AngularJS to Lazy-Loaded Angular ModulesEssential Checklist
Proof that changes are correct
when user is a valid facilitator
Screencast.from.16-02-24.08.09.53.PM.IST.webm
logged out user
Screencast.from.16-02-24.08.25.51.PM.IST.webm
when the user isn't a valid facilitator
Screencast.from.16-02-24.08.27.05.PM.IST.webm
Proof of changes on desktop with slow/throttled network
N/A
Proof of changes on mobile phone
N/A
Proof of changes in Arabic language
N/A
PR Pointers