Skip to content

Initial chromeless template (for LTI Provider) #8225

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.

Sign up for GitHub

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

Merged
cpennington merged 2 commits into from
Jun 2, 2015
Merged

Initial chromeless template (for LTI Provider) #8225

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d88e5b0
Initial chromeless template
May 28, 2015
b4da8e2
Updated LTI render courseware test
simultech Jun 2, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions lms/djangoapps/lti_provider/tests/test_views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,11 +269,9 @@ def test_context(self):
'disable_accordion': True,
'allow_iframing': True,
'disable_header': True,
'disable_footer': True,
'disable_tabs': True,
'staff_access': 'StaffAccess',
'xqa_server': 'http://example.com/xqa',
}
request = build_run_request()
views.render_courseware(request, ALL_PARAMS.copy())
self.render_mock.assert_called_with('courseware/courseware.html', expected_context)
self.render_mock.assert_called_with('courseware/courseware-chromeless.html', expected_context)
4 changes: 1 addition & 3 deletions lms/djangoapps/lti_provider/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,13 +189,11 @@ def render_courseware(request, lti_params):
'disable_accordion': True,
'allow_iframing': True,
'disable_header': True,

@cpennington cpennington May 29, 2015

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.

Perhaps not for this PR, but it would be great if the header were in a block as well, so that the chromeless template could just override that block to be empty, rather than requiring the caller to pass in this variable.

'disable_footer': True,
'disable_tabs': True,
'staff_access': staff,
'xqa_server': settings.FEATURES.get('XQA_SERVER', 'http://example.com/xqa'),
}

return render_to_response('courseware/courseware.html', context)
return render_to_response('courseware/courseware-chromeless.html', context)


def parse_course_and_usage_keys(course_id, usage_id):
Expand Down
159 changes: 159 additions & 0 deletions lms/templates/courseware/courseware-chromeless.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
<%! from django.utils.translation import ugettext as _ %>

@sarina sarina May 28, 2015

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 really dislike the idea of adding in a new base template if we can avoid it, because it means that we now have to keep two templates in sync with changes, and everyone has to understand and know how to test this. Is there a way to not do this?

<%! from django.template.defaultfilters import escapejs %>
<%! from microsite_configuration import page_title_breadcrumbs %>
<%! from edxnotes.helpers import is_feature_enabled as is_edxnotes_enabled %>

@cpennington cpennington May 29, 2015

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.

You can put all of these in one block:

<%!
    from django.utils.translation import ugettext as _
    from django.template.defaultfilters import escapejs
    ...
%>

<%inherit file="/main.html" />
<%namespace name='static' file='/static_content.html'/>
<%def name="course_name()">
<% return _("{course_number} Courseware").format(course_number=course.display_number_with_default) %>

@sarina sarina May 28, 2015

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.

@mcgachey Well my concern is around the actual strings that are included - what if our UX changes and we instead refer to this as "The Course Ware For Course Number {course_number}" in the main template? Do we need to update this template as well? This template looks super similar to courseware/courseware.html, so when there are changes to courseware.html who's responsible for updating courseware-chromeless.html? Sorry if I didn't make my concern clear at first.

@mcgachey mcgachey May 28, 2015

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.

Ah - that might be a different concern than I was thinking of. You're right that any string should only exist in one place (which presumably has implications for translation as well). If they're duplicated in main.html (or somewhere else) then we should rationalize them.

The duplication between this template and courseware/courseware.html is the part that I believe will be straightened out, although it isn't in this change. The duplicated code should be removed from courseware/courseware.html, and this file will be imported there instead. That way there won't be two places to update any changes.

Apologies if I'm putting words into your mouth, Andrew - the time difference could make the conversation somewhat staggered. It might be best to write up a description of how the templates will be structured in order to address the question of duplication.

</%def>

<%block name="bodyclass">courseware ${course.css_class or ''}</%block>
<%block name="title"><title>
% if section_title:
${page_title_breadcrumbs(section_title, course_name())}
% else:
${page_title_breadcrumbs(course_name())}
%endif
</title></%block>

<%block name="header_extras">
% for template_name in ["image-modal"]:
<script type="text/template" id="${template_name}-tpl">
<%static:include path="js/${template_name}.underscore" />
</script>
% endfor
<%
header_file = None
%>
</%block>

<%block name="headextra">
<%static:css group='style-course-vendor'/>
<%static:css group='style-course'/>
## Utility: Notes
% if is_edxnotes_enabled(course):
<%static:css group='style-student-notes'/>
% endif

<%block name="nav_skip">${"#seq_content" if section_title else "#course-content"}</%block>

<%include file="../discussion/_js_head_dependencies.html" />

% if show_chat:
<link rel="stylesheet" href="${static.url('css/vendor/ui-lightness/jquery-ui-1.8.22.custom.css')}" />
## It'd be better to have this in a place like lms/css/vendor/candy,
## but the candy_res/ folder contains images and other junk, and it
## all needs to stay together for the Candy.js plugin to work.
<link rel="stylesheet" href="${static.url('candy_res/candy_full.css')}" />
% endif
${fragment.head_html()}
</%block>

<%block name="js_extra">
<script type="text/javascript" src="${static.url('js/vendor/jquery.scrollTo-1.4.2-min.js')}"></script>
<script type="text/javascript" src="${static.url('js/vendor/flot/jquery.flot.js')}"></script>

## codemirror
<script type="text/javascript" src="${static.url('js/vendor/codemirror-compressed.js')}"></script>

<%static:js group='courseware'/>
<%static:js group='discussion'/>

<%include file="../discussion/_js_body_dependencies.html" />
% if staff_access:
<%include file="xqa_interface.html"/>
% endif

<script type="text/javascript">
var $$course_id = "${course.id | escapejs}";

$(function(){
$(".ui-accordion-header a, .ui-accordion-content .subtitle").each(function() {
var elemText = $(this).text().replace(/^\s+|\s+$/g,''); // Strip leading and trailing whitespace
var wordArray = elemText.split(" ");
var finalTitle = "";
if (wordArray.length > 0) {
for (i=0;i<=wordArray.length-1;i++) {
finalTitle += wordArray[i];
if (i == (wordArray.length-2)) {
finalTitle += "&nbsp;";
} else if (i == (wordArray.length-1)) {
// Do nothing
} else {
finalTitle += " ";
}
}
}
$(this).html(finalTitle);
});
});
</script>

% if show_chat:
<script type="text/javascript" src="${static.url('js/vendor/candy_libs/libs.min.js')}"></script>
<script type="text/javascript" src="${static.url('js/vendor/candy.min.js')}"></script>

<script type="text/javascript">
// initialize the Candy.js plugin
$(document).ready(function() {
Candy.init("http://${chat['domain']}:5280/http-bind/", {
core: { debug: true, autojoin: ["${chat['room']}@conference.${chat['domain']}"] },
view: { resources: "${static.url('candy_res/')}"}
});
Candy.Core.connect("${chat['username']}", "${chat['password']}");

// show/hide the chat widget
$('#chat-toggle').click(function() {
var toggle = $(this);
if (toggle.hasClass('closed')) {
$('#chat-block').show().animate({height: '400px'}, 'slow', function() {
$('#chat-open').hide();
$('#chat-close').show();
});
} else {
$('#chat-block').animate({height: '0px'}, 'slow', function() {
$('#chat-open').show();
$('#chat-close').hide();
$(this).hide(); // do this at the very end
});
}
toggle.toggleClass('closed');
});
});
</script>
% endif

${fragment.foot_html()}

</%block>

<div class="container">
<div class="course-wrapper">
<section class="course-content" id="course-content">
${fragment.body_html()}
</section>
</div>
</div>

<nav class="nav-utilities ${"has-utility-calculator" if course.show_calculator else ""}" aria-label="${_('Course Utilities')}">
## Utility: Chat
% if show_chat:
<%include file="/chat/toggle_chat.html" />
% endif

## Utility: Notes
% if is_edxnotes_enabled(course):
<%include file="/edxnotes/toggle_notes.html" args="course=course"/>
% endif

## Utility: Calc
% if course.show_calculator:
<%include file="/calculator/toggle_calculator.html" />
% endif
</nav>

<%include file="../modal/accessible_confirm.html" />

## No footer in chromeless
<%block name="footer"></%block>