Skip to content

(WIP) Studio library view - Do not merge, for internal review - #1

Closed
antoviaque wants to merge 13 commits into
masterfrom
studio-library-view
Closed

(WIP) Studio library view - Do not merge, for internal review#1
antoviaque wants to merge 13 commits into
masterfrom
studio-library-view

Conversation

@antoviaque

Copy link
Copy Markdown
Member

No description provided.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Why is it implied? You mention this is for the URL - what are the differences?

Also, how do you plan to handle multiple libraries in a single course? library1, library2, etc.? (If we do want it - I remember that the other approach was to create fake courses and handle everything outside of courses)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good questions. What this accomplishes is the following:

If I go to:

  • localhost:8001/course/course-v1:ProblemX+PR0B+2014 - This is the normal course view, showing blocks in the draft-branch branch
  • localhost:8001/library/course-v1:ProblemX+PR0B+2014 - This is the new library view. With the code above, this will look for library data in the library branch by default.
  • localhost:8001/library/course-v1:ProblemX+PR0B+2014+branch@library - This URL is the same thing, but with a CourseLocator string that explicitly specifies the branch. I found it a bit redundant and I thought it looks nicer and is more consistent without the +branch@library part there, so I included this code to make that optional.

If we wanted to allow multiple libraries per course (I don't recommend this though), then each library would have its own branch, and you'd access them like:
localhost:8001/library/course-v1:ProblemX+PR0B+2014+branch@problems-library
localhost:8001/library/course-v1:ProblemX+PR0B+2014+branch@content-library1
localhost:8001/library/course-v1:ProblemX+PR0B+2014+branch@content-library2

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Might be worth waiting for upstream review to see what they think, but calling create_course() on an existing course from create_branch() seem counter-intuitive and a bit hacky -- given that it's a prototype it's fair game, but some refactoring to extract the logic that we need from create_course() could possibly make this cleaner?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That is definitely worth considering. For the prototype combining them was easier, since the create_course() method has a lot of code in it and most of the logic is the same as for creating a branch.

Note: in the future, if we support libraries that are separate from existing courses, we would need logic like:

if course_exists:
    modulestore.create_branch(..., "library", ...)
else:
    modeulestore.create_course(..., "library", ...)

So there you can see how the two methods are somewhat similar. What would probably be ideal would be to move most of the code into create_branch and then have create_course call create_branch - that makes sense and actually reflects what's going on.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I see - it makes sense yep.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Pretty strange yes, curious to know what causes this when/if you find out.

@bradenmacdonald

Copy link
Copy Markdown
Member

Superseded by #5.

@bradenmacdonald
bradenmacdonald deleted the studio-library-view branch October 29, 2014 02:37
e-kolpakov pushed a commit that referenced this pull request May 15, 2015
This fixes a minor layout bug in edx-jsme 1.0. See:
jazkarta/edx-jsme#1

For convenience, the entire diff from the last release is included here:

diff --git a/CHANGES.rst b/CHANGES.rst
new file mode 100644
index 0000000..32e0284
--- /dev/null
+++ b/CHANGES.rst
@@ -0,0 +1,13 @@
+===================
+edX-JSME Change Log
+===================
+
+1.0.1 (2014-09-17)
+------------------
+
+- Fix alignment issue with the correct/incorrect markers. (#1)
+
+1.0 (2014-07-28)
+----------------
+
+- Initial release.
diff --git a/edx_jsme/templates/jsmeinput.html b/edx_jsme/templates/jsmeinput.html
index 861b43f..5ceaf3d 100644
--- a/edx_jsme/templates/jsmeinput.html
+++ b/edx_jsme/templates/jsmeinput.html
@@ -38,6 +38,7 @@
     waitfor=""
     value="${value|h}"/>

+    <br/>
     <p id="answer_${id}" class="answer"></p>

     <p class="status">
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.

2 participants