Content libraries paging #9
Conversation
|
|
| </script> | ||
| % endfor | ||
|
|
||
| <div class="xblock-container-paging-parameters" data-start="${first_displayed}" data-displayed="${displayed_children}" data-total="${total_children}"/> |
There was a problem hiding this comment.
I don't like the idea of passing data through data attributes in rendered html, but it's a workaround that allows paging to work. Otherwise some invasive modifications need to be made to the way child XBlocks are delivered to studio
There was a problem hiding this comment.
Yeah I see what you mean. The only thing I can suggest for now is changing the end of this line slightly since <div /> is not valid HTML5.
|
Very nice work @e-kolpakov! Seems to work very well. Comments made inline. The only issue I encountered is just missing functionality: right now if I go to page 3 then refresh, I find myself back on page 1. I think you should use As for how you chose to implement this overall, I think it's good, but we'll have to see what upstream says :) |
|
@bradenmacdonald thanks for the review! I've addressed those inline review notes. As of history - unfortunately there are no Backbone routing present at where we are, so we either need to implement backbone routing or do history manipulations using plain HTML5 API. I've spent about an hour implementing HTML5 version. In the end it was capable of tracking history and handling back button, but with bugs and zero test coverage. So I decided that it deserves a separate task. |
|
@e-kolpakov Sounds good :) |
bradenmacdonald
force-pushed
the
content-libraries-prototype
branch
from
3e44683
to
1c5c007
Compare
…ering + scrolling to last page on children add.
… + styled the same way as File upload paging
Implemented paging re-render on removing chuildren XBlock
…d appears to be broken)
…paging support in LibraryXBlock
bradenmacdonald
force-pushed
the
content-libraries-paging
branch
from
e1bcc0b
to
7372aa6
Compare
|
This will need to be rebased onto content_libraries/2-studio-lib-support. The rebase will be complicated by the fact that I changed the Once PR 6046 has been merged, we will open an upstream PR to merge this branch into the |
|
@bradenmacdonald just one question is content_libraries/2-studio-lib-support ready for rebasing? I mean, since it's a base for PR 6046, there will be more modifications to it while you address the comments from upstream review, so it might make sense to hold off rebasing this branch until PR 6046 is merged, to reduce the number of rebases. |
|
@e-kolpakov Yes, you might want to wait until PR 6046 is merged. Up to you. |
* Adds actual number of assignments types assigned to. * Adds total weight validation on assignment types. * Adds Assignment type weight summary modal dialog. * Adds translated strings (he, ar)
…em-nltk Install chem nltk from edX fork, and fix django-wiki dependency. Rue89 (cherry picked from commit e3297b4)
No description provided.