-
Notifications
You must be signed in to change notification settings - Fork 3.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP Refactored schedule tree #11647
WIP Refactored schedule tree #11647
Conversation
Thanks for the pull request, @amir-qayyum-khan! I've created OSPR-1161 to keep track of it in JIRA. JIRA is a place for product owners to prioritize feature reviews by the engineering development teams. Feel free to add as much of the following information to the ticket:
All technical communication about the code itself will still be done via the GitHub pull request interface. As a reminder, our process documentation is here. |
dc6b19a
to
02be0e3
Compare
FTR this will need both T&L and a11y review @cahrens @cptvitamin @scottrish |
@cahrens @cptvitamin @scottrish any sense when you will be able to get to this? |
@cahrens @cptvitamin @scottrish any sense of a timeline? |
This has not been prioritized into a sprint yet (so for TNL, unfortunately no). |
@amir-qayyum-khan TNL will review this PR within the next 2 weeks (reviewer TBD). Please let us know if you will not be available in the next 2 weeks for questions and feedback. Note that this PR will additionally require accessibility review. |
Thanks for the reminder @cahrens - @cptvitamin just a ping on the a11y review on this one as well |
@cahrens thanks for reminder. |
02be0e3
to
ec5d71f
Compare
I have created a sandbox with this branch of code-- https://ccx.sandbox.edx.org. @amir-qayyum-khan how do I enable the CCX functionality? |
2213f5f
to
8f56df0
Compare
jenkins run lettuce |
8f56df0
to
b2dec8d
Compare
@@ -0,0 +1,181 @@ | |||
<div class="field"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
XSS best practices need to be followed-- there are unescaped strings in here like "chapter.display_name" and "date" (in template above). All strings should be escaped unless they include HTML. If they include HTML, they must be properly wrapped using HTMLUtils methods.
Please read over the new XSS best practices document-- http://edx.readthedocs.org/projects/edx-developer-guide/en/latest/conventions/safe_templates.html
All the underscore files in this PR should be cleaned up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #11899-- some of the templates may have been fixed up already. Although it looks like underscore files where not touched.
@cahrens i will complete work on it this week. I will tag you as soon as i complete. |
@amir-qayyum-khan Can I get an update on when this will be ready for review? Thanks! |
900be4c
to
eabd4ea
Compare
@cahrens Do you know case of this error?
|
It's an error running the RequireJS optimizer. The message doesn't give any details-- you will have to re-examine changes you've made related to JS optimization. |
And it looks like it is too late, but I'd like to plead again that you don't squash review feedback commits with the original commit. Please do squash the feedback commits into a single commit (since my last review), but don't squash with the original commit as then I have no way to tell what has change since I last looked at the PR. |
b4c51c6
to
8b11f3b
Compare
jenkins run bokchoy |
8b11f3b
to
b3002cd
Compare
8e55d40
to
61cb209
Compare
cc @pdpinch |
61cb209
to
4615ee9
Compare
jenkins run quality |
4615ee9
to
a09aa84
Compare
a09aa84
to
44be531
Compare
jenkins run a11y |
@amir-qayyum-khan We're going to be closing OSPRs that have been 'waiting for author' for more than three weeks with no activity. If you're not sure why this is in status 'waiting for author' please ask. Please also feel free to reopen this PR if/when you're ready to update it. |
Background
fixes mitocw#146, fixes mitocw#165, fixes mitocw#54, fixes mitocw#39
mit_pr
What is done in this PR
Studio Updates: None.
LMS Updates:
Detail:
hidden=false
is shown to schedule tree on left size, the units withhidden=true
is supplied to form at right side. From there user can change visibility of hidden units by clickingAdding a unit
orAdding all units
@pdpinch @giocalitri @justinabrahms