Skip to content

Add new container page that can display nested xblocks - #2539

Merged
andy-armstrong merged 1 commit into
masterfrom
andya/nested-xblocks
Feb 27, 2014
Merged

Add new container page that can display nested xblocks#2539
andy-armstrong merged 1 commit into
masterfrom
andya/nested-xblocks

Conversation

@andy-armstrong

Copy link
Copy Markdown
Contributor

This change introduces a new container page which will eventually replace the current unit page for STUD-1244. This new page is able to display nested xblocks, and in particular can show nested child verticals as collapsible sections inline.

In addition, the unit page has been updated to link all child containers over to the new container page. This makes it possible for the user to view the details of these nested xblocks. Note that the new page is currently only read-only, and a subsequent story will add editing support.

The design can be found on the wiki: Provide a new read-only view for container XBlocks

@andy-armstrong

Copy link
Copy Markdown
Contributor Author

@mhoeber This PR isn't ready to go quite yet, but feel free to try it out and ask myself or Marco questions.

@marcotuts

Copy link
Copy Markdown
Contributor

@talbs, @frrrances - This is the PR that will land after the styling PR I've already tagged you on. This is the dynamic templated version of the container page using the styling and reference page from the earlier PR.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't forget to update the documentation.

@andy-armstrong

Copy link
Copy Markdown
Contributor Author

@cahrens @cpennington please review this when you have a chance. Note that the FED still isn't 100% wrapped up (that work is happening separately on https://github.com/edx/edx-platform/pull/2507), and there are some unexpected unit test failures in other areas that I still have to look into.

@singingwolfboy if you are back in action, I'd love your feedback too.

@marcotuts @talbs @frrrances you may want to try this out too.

Comment thread cms/templates/unit.html

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain these new dependencies?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved these from module_edit.coffee into the pages themselves, as it felt odd to me that particular views would be responsible for choosing which xblock runtime to use. It seemed more natural to me that each page would be responsible for that decision. Ideally there would be a common 'app' class that would be responsible, but I don't think we have something like that right now (or maybe I missed it).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about xmodule and coffee/src/main?

I think there is a common app place that you can put the xblock dependency-- right in the RequireJS configuration. @singingwolfboy what do you think?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @cahrens -- @andy-armstrong, can you explain why you're adding these dependencies? Especially since you're not binding them to variables in the function call, so they appear to be unused...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants