From 3fd9fa6415925154ec8e1fe3c656c813cc25470a Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Wed, 14 Jan 2015 19:39:19 -0800 Subject: [PATCH 1/3] Support for inline explanatory popups in problem XML --- common/lib/capa/capa/customrender.py | 34 ++++++- .../capa/capa/templates/clarification.html | 4 + .../lib/xmodule/xmodule/css/capa/display.scss | 6 ++ common/static/js/spec/tooltip_manager_spec.js | 6 ++ common/static/js/src/tooltip_manager.js | 14 ++- common/test/acceptance/pages/lms/problem.py | 16 ++++ .../tests/lms/test_lms_matlab_problem.py | 51 +++-------- .../acceptance/tests/lms/test_lms_problems.py | 88 +++++++++++++++++++ 8 files changed, 178 insertions(+), 41 deletions(-) create mode 100644 common/lib/capa/capa/templates/clarification.html create mode 100644 common/test/acceptance/tests/lms/test_lms_problems.py diff --git a/common/lib/capa/capa/customrender.py b/common/lib/capa/capa/customrender.py index bc80d270f4..8b42b76bf5 100644 --- a/common/lib/capa/capa/customrender.py +++ b/common/lib/capa/capa/customrender.py @@ -6,8 +6,6 @@ These tags do not have state, so they just get passed the system (for access to and the xml element. """ -from .registry import TagRegistry - import logging import re @@ -137,3 +135,35 @@ class TargetedFeedbackRenderer(object): return xhtml registry.register(TargetedFeedbackRenderer) + +#----------------------------------------------------------------------------- + + +class ClarificationRenderer(object): + """ + A clarification appears as an inline icon which reveals more information when the user + hovers over it. + + e.g.

Enter the ROA Return on Assets for 2015:

+ """ + tags = ['clarification'] + + def __init__(self, system, xml): + self.system = system + # Get any text content found inside this tag prior to the first child tag. It may be a string or None type. + initial_text = xml.text if xml.text else '' + self.inner_html = initial_text + ''.join(etree.tostring(element) for element in xml) # pylint: disable=no-member + self.tail = xml.tail + + def get_html(self): + """ + Return the contents of this tag, rendered to html, as an etree element. + """ + context = {'clarification': self.inner_html} + html = self.system.render_template("clarification.html", context) + xml = etree.XML(html) # pylint: disable=no-member + # We must include any text that was following our original ... XML node.: + xml.tail = self.tail + return xml + +registry.register(ClarificationRenderer) diff --git a/common/lib/capa/capa/templates/clarification.html b/common/lib/capa/capa/templates/clarification.html new file mode 100644 index 0000000000..e71de94ccd --- /dev/null +++ b/common/lib/capa/capa/templates/clarification.html @@ -0,0 +1,4 @@ + + + (${clarification}) + diff --git a/common/lib/xmodule/xmodule/css/capa/display.scss b/common/lib/xmodule/xmodule/css/capa/display.scss index 4bd9368d8c..303a38a5d5 100644 --- a/common/lib/xmodule/xmodule/css/capa/display.scss +++ b/common/lib/xmodule/xmodule/css/capa/display.scss @@ -162,6 +162,12 @@ div.problem { white-space: nowrap; overflow: hidden; } + span.clarification i { + font-style: normal; + &:hover { + color: $blue; + } + } } &.unanswered { diff --git a/common/static/js/spec/tooltip_manager_spec.js b/common/static/js/spec/tooltip_manager_spec.js index d29d44281d..e4f0234489 100644 --- a/common/static/js/spec/tooltip_manager_spec.js +++ b/common/static/js/spec/tooltip_manager_spec.js @@ -70,6 +70,12 @@ describe('TooltipManager', function () { expect($('.tooltip')).toBeHidden(); }); + it('can be configured to show when user clicks on the element', function () { + this.element.attr('data-tooltip-show-on-click', true); + this.element.trigger($.Event("click")); + expect($('.tooltip')).toBeVisible(); + }); + it('should moves correctly', function () { showTooltip(this.element); expect($('.tooltip')).toBeVisible(); diff --git a/common/static/js/src/tooltip_manager.js b/common/static/js/src/tooltip_manager.js index 9fd0f14052..ae7bc6e87c 100644 --- a/common/static/js/src/tooltip_manager.js +++ b/common/static/js/src/tooltip_manager.js @@ -26,7 +26,7 @@ 'mouseover.TooltipManager': this.showTooltip, 'mousemove.TooltipManager': this.moveTooltip, 'mouseout.TooltipManager': this.hideTooltip, - 'click.TooltipManager': this.hideTooltip + 'click.TooltipManager': this.click }, this.SELECTOR); }, @@ -68,6 +68,18 @@ this.tooltipTimer = setTimeout(this.hide, 50); }, + click: function(event) { + var showOnClick = !!$(event.currentTarget).data('tooltip-show-on-click'); // Default is false + if (showOnClick) { + this.show(); + if (this.tooltipTimer) { + clearTimeout(this.tooltipTimer); + } + } else { + this.hideTooltip(event); + } + }, + destroy: function () { this.tooltip.remove(); // Unbind all delegated event handlers in the ".TooltipManager" diff --git a/common/test/acceptance/pages/lms/problem.py b/common/test/acceptance/pages/lms/problem.py index 9e31ada7d2..239dff45de 100644 --- a/common/test/acceptance/pages/lms/problem.py +++ b/common/test/acceptance/pages/lms/problem.py @@ -46,3 +46,19 @@ class ProblemPage(PageObject): Is there a "correct" status showing? """ return self.q(css="div.problem div.capa_inputtype.textline div.correct p.status").is_present() + + def click_clarification(self, index=0): + """ + Click on an inline icon that can be included in problem text using an HTML element: + + Problem clarification text hidden by an icon in rendering Text + """ + self.q(css='div.problem .clarification:nth-child({index}) i[data-tooltip]'.format(index=index + 1)).click() + + @property + def visible_tooltip_text(self): + """ + Get the text seen in any tooltip currently visible on the page. + """ + self.wait_for_element_visibility('body > .tooltip', 'A tooltip is visible.') + return self.q(css='body > .tooltip').text[0] diff --git a/common/test/acceptance/tests/lms/test_lms_matlab_problem.py b/common/test/acceptance/tests/lms/test_lms_matlab_problem.py index f62d021bb5..43847c7f7e 100644 --- a/common/test/acceptance/tests/lms/test_lms_matlab_problem.py +++ b/common/test/acceptance/tests/lms/test_lms_matlab_problem.py @@ -1,38 +1,24 @@ # -*- coding: utf-8 -*- """ -End-to-end tests for the LMS. +Test for matlab problems """ import time -from ..helpers import UniqueCourseTest -from ...pages.studio.auto_auth import AutoAuthPage -from ...pages.lms.courseware import CoursewarePage from ...pages.lms.matlab_problem import MatlabProblemPage -from ...fixtures.course import CourseFixture, XBlockFixtureDesc +from ...fixtures.course import XBlockFixtureDesc from ...fixtures.xqueue import XQueueResponseFixture +from .test_lms_problems import ProblemsTest from textwrap import dedent -class MatlabProblemTest(UniqueCourseTest): +class MatlabProblemTest(ProblemsTest): """ Tests that verify matlab problem "Run Code". """ - USERNAME = "STAFF_TESTER" - EMAIL = "johndoe@example.com" - - def setUp(self): - super(MatlabProblemTest, self).setUp() - - self.XQUEUE_GRADE_RESPONSE = None - - self.courseware_page = CoursewarePage(self.browser, self.course_id) - - # Install a course with sections/problems, tabs, updates, and handouts - course_fix = CourseFixture( - self.course_info['org'], self.course_info['number'], - self.course_info['run'], self.course_info['display_name'] - ) - + def get_problem(self): + """ + Create a matlab problem for the test. + """ problem_data = dedent(""" @@ -62,18 +48,7 @@ class MatlabProblemTest(UniqueCourseTest): """) - - course_fix.add_children( - XBlockFixtureDesc('chapter', 'Test Section').add_children( - XBlockFixtureDesc('sequential', 'Test Subsection').add_children( - XBlockFixtureDesc('problem', 'Test Matlab Problem', data=problem_data) - ) - ) - ).install() - - # Auto-auth register for the course. - AutoAuthPage(self.browser, username=self.USERNAME, email=self.EMAIL, - course_id=self.course_id, staff=False).visit() + return XBlockFixtureDesc('problem', 'Test Matlab Problem', data=problem_data) def _goto_matlab_problem_page(self): """ @@ -92,13 +67,13 @@ class MatlabProblemTest(UniqueCourseTest): # Enter a submission, which will trigger a pre-defined response from the XQueue stub. self.submission = "a=1" + self.unique_id[0:5] - self.XQUEUE_GRADE_RESPONSE = {'msg': self.submission} + self.xqueue_grade_response = {'msg': self.submission} matlab_problem_page = self._goto_matlab_problem_page() # Configure the XQueue stub's response for the text we will submit - if self.XQUEUE_GRADE_RESPONSE is not None: - XQueueResponseFixture(self.submission, self.XQUEUE_GRADE_RESPONSE).install() + if self.xqueue_grade_response is not None: + XQueueResponseFixture(self.submission, self.xqueue_grade_response).install() matlab_problem_page.set_response(self.submission) matlab_problem_page.click_run_code() @@ -113,6 +88,6 @@ class MatlabProblemTest(UniqueCourseTest): self.assertEqual(u'', matlab_problem_page.get_grader_msg(".external-grader-message")[0]) self.assertEqual( - self.XQUEUE_GRADE_RESPONSE.get("msg"), + self.xqueue_grade_response.get("msg"), matlab_problem_page.get_grader_msg(".ungraded-matlab-result")[0] ) diff --git a/common/test/acceptance/tests/lms/test_lms_problems.py b/common/test/acceptance/tests/lms/test_lms_problems.py new file mode 100644 index 0000000000..b19f65c10d --- /dev/null +++ b/common/test/acceptance/tests/lms/test_lms_problems.py @@ -0,0 +1,88 @@ +# -*- coding: utf-8 -*- +""" +Bok choy acceptance tests for problems in the LMS + +See also old lettuce tests in lms/djangoapps/courseware/features/problems.feature +""" +from ..helpers import UniqueCourseTest +from ...pages.studio.auto_auth import AutoAuthPage +from ...pages.lms.courseware import CoursewarePage +from ...pages.lms.problem import ProblemPage +from ...fixtures.course import CourseFixture, XBlockFixtureDesc +from textwrap import dedent + + +class ProblemsTest(UniqueCourseTest): + """ + Base class for tests of problems in the LMS. + """ + USERNAME = "joe_student" + EMAIL = "joe@example.com" + + def setUp(self): + super(ProblemsTest, self).setUp() + + self.xqueue_grade_response = None + + self.courseware_page = CoursewarePage(self.browser, self.course_id) + + # Install a course with a hierarchy and problems + course_fixture = CourseFixture( + self.course_info['org'], self.course_info['number'], + self.course_info['run'], self.course_info['display_name'] + ) + + problem = self.get_problem() + course_fixture.add_children( + XBlockFixtureDesc('chapter', 'Test Section').add_children( + XBlockFixtureDesc('sequential', 'Test Subsection').add_children(problem) + ) + ).install() + + # Auto-auth register for the course. + AutoAuthPage(self.browser, username=self.USERNAME, email=self.EMAIL, + course_id=self.course_id, staff=False).visit() + + def get_problem(self): + """ Subclasses should override this to complete the fixture """ + raise NotImplementedError() + + +class ProblemClarificationTest(ProblemsTest): + """ + Tests the element that can be used in problem XML. + """ + def get_problem(self): + """ + Create a problem with a + """ + xml = dedent(""" + + +

+ Given the data in Table 7 Table 7: "Example PV Installation Costs", + Page 171 of Roberts textbook, compute the ROI + Return on Investment (per year) over 20 years. +

+ + + + + + """) + return XBlockFixtureDesc('problem', 'TOOLTIP TEST PROBLEM', data=xml) + + def test_clarification(self): + """ + Test that we can see the tooltips. + """ + self.courseware_page.visit() + problem_page = ProblemPage(self.browser) + self.assertEqual(problem_page.problem_name, 'TOOLTIP TEST PROBLEM') + problem_page.click_clarification(0) + self.assertIn('"Example PV Installation Costs"', problem_page.visible_tooltip_text) + problem_page.click_clarification(1) + tooltip_text = problem_page.visible_tooltip_text + self.assertIn('Return on Investment', tooltip_text) + self.assertIn('per year', tooltip_text) + self.assertNotIn('strong', tooltip_text) From e833d9c2e809772a169aa2a50a9e0a374d2b99ef Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Thu, 5 Feb 2015 15:49:46 -0800 Subject: [PATCH 2/3] Accessibility improvements --- common/lib/capa/capa/templates/clarification.html | 5 +++-- common/lib/xmodule/xmodule/js/src/capa/display.coffee | 11 +++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/common/lib/capa/capa/templates/clarification.html b/common/lib/capa/capa/templates/clarification.html index e71de94ccd..6f730cfac5 100644 --- a/common/lib/capa/capa/templates/clarification.html +++ b/common/lib/capa/capa/templates/clarification.html @@ -1,4 +1,5 @@ - - + + (${clarification}) diff --git a/common/lib/xmodule/xmodule/js/src/capa/display.coffee b/common/lib/xmodule/xmodule/js/src/capa/display.coffee index 0ff252568a..fdaafd4c51 100644 --- a/common/lib/xmodule/xmodule/js/src/capa/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/capa/display.coffee @@ -34,6 +34,17 @@ class @Problem @$('div.action input.reset').click @reset @$('div.action button.show').click @show @$('div.action input.save').click @save + # Accessibility helper for sighted keyboard users to show tooltips on focus: + @$('.clarification').focus (ev) => + icon = $(ev.target).children "i" + iconPos = icon.offset() + fakeEvent = jQuery.Event "mouseover", { + pageX: iconPos.left + icon.width()/2, + pageY: iconPos.top + icon.height()/2 + } + icon.trigger(fakeEvent).trigger "click" + @$('.clarification').blur (ev) => + $(ev.target).children("i").trigger "mouseout" @bindResetCorrectness() From d12e173c668fc5719e104ac1593e37d9121ab6e7 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Fri, 6 Feb 2015 09:39:37 -0800 Subject: [PATCH 3/3] Change TooltipManager API so we can remove fake event --- .../xmodule/js/src/capa/display.coffee | 9 ++----- common/static/js/spec/tooltip_manager_spec.js | 5 ++++ common/static/js/src/tooltip_manager.js | 26 ++++++++++++++----- 3 files changed, 27 insertions(+), 13 deletions(-) diff --git a/common/lib/xmodule/xmodule/js/src/capa/display.coffee b/common/lib/xmodule/xmodule/js/src/capa/display.coffee index fdaafd4c51..f18963b9ba 100644 --- a/common/lib/xmodule/xmodule/js/src/capa/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/capa/display.coffee @@ -37,14 +37,9 @@ class @Problem # Accessibility helper for sighted keyboard users to show tooltips on focus: @$('.clarification').focus (ev) => icon = $(ev.target).children "i" - iconPos = icon.offset() - fakeEvent = jQuery.Event "mouseover", { - pageX: iconPos.left + icon.width()/2, - pageY: iconPos.top + icon.height()/2 - } - icon.trigger(fakeEvent).trigger "click" + window.globalTooltipManager.openTooltip icon @$('.clarification').blur (ev) => - $(ev.target).children("i").trigger "mouseout" + window.globalTooltipManager.hide() @bindResetCorrectness() diff --git a/common/static/js/spec/tooltip_manager_spec.js b/common/static/js/spec/tooltip_manager_spec.js index e4f0234489..c4011b1f95 100644 --- a/common/static/js/spec/tooltip_manager_spec.js +++ b/common/static/js/spec/tooltip_manager_spec.js @@ -76,6 +76,11 @@ describe('TooltipManager', function () { expect($('.tooltip')).toBeVisible(); }); + it('can be be triggered manually', function () { + this.tooltip.openTooltip(this.element); + expect($('.tooltip')).toBeVisible(); + }); + it('should moves correctly', function () { showTooltip(this.element); expect($('.tooltip')).toBeVisible(); diff --git a/common/static/js/src/tooltip_manager.js b/common/static/js/src/tooltip_manager.js index ae7bc6e87c..83eba8cad3 100644 --- a/common/static/js/src/tooltip_manager.js +++ b/common/static/js/src/tooltip_manager.js @@ -46,17 +46,31 @@ }, showTooltip: function(event) { - var tooltipText = $(event.currentTarget).attr('data-tooltip'); - this.tooltip - .html(tooltipText) - .css(this.getCoords(event.pageX, event.pageY)); - + this.prepareTooltip(event.currentTarget, event.pageX, event.pageY); if (this.tooltipTimer) { clearTimeout(this.tooltipTimer); } this.tooltipTimer = setTimeout(this.show, 500); }, + prepareTooltip: function(element, pageX, pageY) { + pageX = typeof pageX !== 'undefined' ? pageX : element.offset().left + element.width()/2; + pageY = typeof pageY !== 'undefined' ? pageY : element.offset().top + element.height()/2; + var tooltipText = $(element).attr('data-tooltip'); + this.tooltip + .html(tooltipText) + .css(this.getCoords(pageX, pageY)); + }, + + // To manually trigger a tooltip to reveal, other than through user mouse movement: + openTooltip: function(element) { + this.prepareTooltip(element); + this.show(); + if (this.tooltipTimer) { + clearTimeout(this.tooltipTimer); + } + }, + moveTooltip: function(event) { this.tooltip.css(this.getCoords(event.pageX, event.pageY)); }, @@ -90,6 +104,6 @@ window.TooltipManager = TooltipManager; $(document).ready(function () { - new TooltipManager(document.body); + window.globalTooltipManager = new TooltipManager(document.body); }); }());