Several style fixes per review feedback.

poll/poll.py

@@ -47,33 +47,27 @@ class PollBlock(XBlock):
             'total': total, 'feedback': process_markdown(self.feedback),
         }
 
-    def get_tally(self):
+    def clean_tally(self):
         """
-        Grabs the Tally and cleans it up, if necessary. Scoping prevents us from
-        modifying this in the studio and in the LMS the way we want to without
-        undesirable side effects. So we just clean it up on first access within
-        the LMS, in case the studio has made changes to the answers.
+        Cleans the tally. Scoping prevents us from modifying this in the studio
+        and in the LMS the way we want to without undesirable side effects. So
+        we just clean it up on first access within the LMS, in case the studio
+        has made changes to the answers.
         """
-        tally = self.tally
         answers = OrderedDict(self.answers)
         for key in answers.keys():
-            if key not in tally:
-                tally[key] = 0
+            if key not in self.tally:
+                self.tally[key] = 0
 
-        for key in tally.keys():
+        for key in self.tally.keys():
             if key not in answers:
-                del tally[key]
-
-        return tally
+                del self.tally[key]
 
     def any_image(self):
         """
         Find out if any answer has an image, since it affects layout.
         """
-        for value in dict(self.answers).values():
-            if value['img']:
-                return True
-        return False
+        return any(value['img'] for value in dict(self.answers).values())
 
     def tally_detail(self):
         """
@@ -83,11 +77,13 @@ class PollBlock(XBlock):
         answers = OrderedDict(self.answers)
         choice = self.get_choice()
         total = 0
-        source_tally = self.get_tally()
+        self.clean_tally()
+        source_tally = self.tally
         any_img = self.any_image()
         for key, value in answers.items():
+            count = int(source_tally[key])
             tally.append({
-                'count': int(source_tally[key]),
+                'count': count,
                 'answer': value['label'],
                 'img': value['img'],
                 'key': key,
@@ -96,16 +92,13 @@ class PollBlock(XBlock):
                 'last': False,
                 'any_img': any_img,
             })
-            total += tally[-1]['count']
+            total += count
 
         for answer in tally:
             try:
-                percent = (answer['count'] / float(total))
-                answer['percent'] = int(percent * 100)
+                answer['percent'] = int(answer['count'] / float(total)) * 100
                 if answer['key'] == choice:
                     answer['choice'] = True
-                if answer['img']:
-                    any_img = True
             except ZeroDivisionError:
                 answer['percent'] = 0
 
@@ -129,7 +122,18 @@ class PollBlock(XBlock):
         else:
             return None
 
-    # TO-DO: change this view to display your data your own way.
+    def create_fragment(self, context, template, css, js, js_init):
+        html = Template(
+            self.resource_string(template)).render(Context(context))
+        frag = Fragment(html)
+        frag.add_javascript_url(
+            self.runtime.local_resource_url(
+                self, 'public/js/vendor/handlebars.js'))
+        frag.add_css(self.resource_string(css))
+        frag.add_javascript(self.resource_string(js))
+        frag.initialize_js(js_init)
+        return frag
+
     def student_view(self, context=None):
         """
         The primary view of the PollBlock, shown to students
@@ -159,23 +163,20 @@ class PollBlock(XBlock):
             detail, total = self.tally_detail()
             context.update({'tally': detail, 'total': total})
 
-        context = Context(context)
-        html = self.resource_string("public/html/poll.html")
-        html = Template(html).render(context)
-        frag = Fragment(html)
-        frag.add_css(self.resource_string("public/css/poll.css"))
-        frag.add_javascript_url(
-            self.runtime.local_resource_url(
-                self, 'public/js/vendor/handlebars.js'))
-        frag.add_javascript(self.resource_string("public/js/poll.js"))
-        frag.initialize_js('PollBlock')
-        return frag
+        return self.create_fragment(
+            context, "public/html/poll.html", "public/css/poll.css",
+            "public/js/poll.js", "PollBlock")
 
     @XBlock.json_handler
     def load_answers(self, data, suffix=''):
-        return {'answers': [{'key': key, 'text': value['label'], 'img': value['img']}
-                            for key, value in self.answers
-        ]}
+        return {
+            'answers': [
+                {
+                    'key': key, 'text': value['label'], 'img': value['img']
+                }
+                for key, value in self.answers
+            ]
+        }
 
     def studio_view(self, context=None):
         if not context:
@@ -187,18 +188,9 @@ class PollBlock(XBlock):
             'feedback': self.feedback,
             'js_template': js_template
         })
-        context = Context(context)
-        html = self.resource_string("public/html/poll_edit.html")
-        html = Template(html).render(context)
-        frag = Fragment(html)
-        frag.add_javascript_url(
-            self.runtime.local_resource_url(
-                self, 'public/js/vendor/handlebars.js'))
-        frag.add_css(self.resource_string('/public/css/poll_edit.css'))
-        frag.add_javascript(self.resource_string("public/js/poll_edit.js"))
-        frag.initialize_js('PollEditBlock')
-
-        return frag
+        return self.create_fragment(
+            context, "public/html/poll_edit.html",
+            "/public/css/poll_edit.css", "public/js/poll_edit.js", "PollEditBlock")
 
     @XBlock.json_handler
     def studio_submit(self, data, suffix=''):
@@ -209,7 +201,7 @@ class PollBlock(XBlock):
             result['success'] = False
         else:
             question = data['question'][:4096]
-        if 'feedback' not in data or not data['feedback']:
+        if 'feedback' not in data or not data['feedback'].strip():
             feedback = ''
         else:
             feedback = data['feedback'][:4096]
@@ -293,20 +285,15 @@ class PollBlock(XBlock):
             result['errors'].append('No key "{choice}" in answers table.'.format(choice=choice))
             return result
 
-        tally = self.get_tally()
+        self.clean_tally()
         self.choice = choice
-        running_total = tally.get(choice, 0)
-        tally[choice] = running_total + 1
+        self.tally[choice] = self.tally.get(choice, 0) + 1
         # Let the LMS know the user has answered the poll.
         self.runtime.publish(self, 'progress', {})
         result['success'] = True
 
-        self.tally = tally
-
         return result
 
-    # TO-DO: change this to create the scenarios you'd like to see in the
-    # workbench while developing your XBlock.
     @staticmethod
     def workbench_scenarios():
         """

poll/public/README.txt

-This static directory is for files that should be included in your kit as plain
-static files.
-
-You can ask the runtime for a URL that will retrieve these files with:
-
-    url = self.runtime.local_resource_url(self, "static/js/lib.js")
-
-The default implementation is very strict though, and will not serve files from
-the static directory.  It will serve files from a directory named "public".
-Create a directory alongside this one named "public", and put files there.
-Then you can get a url with code like this:
-
-    url = self.runtime.local_resource_url(self, "public/js/lib.js")
-
-The sample code includes a function you can use to read the content of files
-in the static directory, like this:
-
-    frag.add_javascript(self.resource_string("static/js/my_block.js"))

poll/public/css/poll.css

+/* CSS for PollBlock Student View */
+
 .poll-answer {
     margin-left: 1em;
     font-weight: bold;

poll/public/css/poll_edit.css

-/* CSS for PollBlock */
+/* CSS for PollBlock Studio Menu View */
 
 .poll-delete-answer {
     float: right;
@@ -21,11 +21,6 @@
     padding: 10px;
 }
 
-label.poll-label {
-    font-weight: bold;
-    font-size: 16pt;
-}
-
 .poll-move-up {
     opacity: .5;
 }

poll/public/handlebars/results.handlebars

@@ -7,7 +7,7 @@
               <input id="answer-{{key}}" type="radio" disabled {{#if choice}}checked="True"{{/if}} />
             </div>
             {{#if any_img}}
-                <div class="poll-image">
+                <div class="poll-image result-image">
                     <label for="answer-{{key}}" class="poll-image-label">
                         {{#if img}}
                           <img src="{{img}}" />

poll/public/handlebars/studio.handlebars

@@ -12,7 +12,7 @@
         </div>
     </div>
     <span class="tip setting-help">Enter an answer for the user to select. An answer must have an image URL or text, and can have both.</span>
-    <a href="#" class="button action-button poll-delete-answer" onclick="return false;">Delete</a>
+    <a href="#" class="button action-button poll-delete-answer">Delete</a>
 </li>
 {{/each}}
 </script>
\ No newline at end of file

poll/public/html/poll_edit.html

@@ -34,10 +34,10 @@
     <div class="xblock-actions">
         <ul>
             <li class="action-item" id="poll-add-answer">
-                <a href="#" class="button action-button" onclick="return false;">Add Answer</a>
+                <a href="#" class="button action-button" class="poll-add-answer-link">Add Answer</a>
             </li>
             <li class="action-item">
-                <input type="submit" class="button action-primary save-button" value="Save" onclick="return false;" />
+                <input type="submit" class="button action-primary save-button" value="Save" />
             </li>
             <li class="action-item">
                 <a href="#" class="button cancel-button">Cancel</a>

poll/public/js/poll.js

@@ -22,12 +22,20 @@ function PollBlock(runtime, element) {
         })
     }
 
+
+    function enableSubmit() {
+        submit.removeAttr("disabled");
+        answers.unbind("change.EnableSubmit");
+    }
+
+    // If the submit button doesn't exist, the user has already
+    // selected a choice.
     if (submit.length) {
-        var radios = $('input[name=choice]:checked', element);
+        var radio = $('input[name=choice]:checked', element);
         submit.click(function (event) {
             // Refresh.
-            radios = $(radios.selector, element);
-            var choice = radios.val();
+            radio = $(radio.selector, element);
+            var choice = radio.val();
             $.ajax({
                 type: "POST",
                 url: voteUrl,
@@ -35,12 +43,10 @@ function PollBlock(runtime, element) {
                 success: getResults
             });
         });
-        var answers = $('li', element);
-        function enableSubmit() {
-            submit.removeAttr("disabled");
-            answers.unbind("change.EnableSubmit");
-        }
-        if (! radios.val()) {
+        // If the user has refreshed the page, they may still have an answer
+        // selected and the submit button should be enabled.
+        var answers = $('input[type=radio]', element);
+        if (! radio.val()) {
             answers.bind("change.EnableSubmit", enableSubmit);
         } else {
             enableSubmit();
@@ -48,8 +54,4 @@ function PollBlock(runtime, element) {
     } else {
         getResults({'success': true});
     }
-
-    $(function ($) {
-
-    });
 }
\ No newline at end of file

poll/public/js/poll_edit.js

@@ -4,42 +4,35 @@ function PollEditBlock(runtime, element) {
     var answerTemplate = Handlebars.compile(temp);
     var pollLineItems =$('#poll-line-items', element);
 
-    // We probably don't need something this complex, but UUIDs are the
-    // standard.
-    function generateUUID(){
-        var d = new Date().getTime();
-        return 'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx'.replace(/[xy]/g, function(c) {
-            var r = (d + Math.random()*16)%16 | 0;
-            d = Math.floor(d/16);
-            return (c=='x' ? r : (r&0x7|0x8)).toString(16);
-        })
-    }
-
     function empowerDeletes(scope) {
         $('.poll-delete-answer', scope).click(function () {
             $(this).parent().remove();
         });
     }
 
-    // Above this point are other settings.
+    /*
+    The poll answers need to be reorderable. As the UL they are in is not
+    easily isolated, we need to start checking their position to make
+    sure they aren't ordered above the other settings, which are also
+    in the list.
+    */
     var starting_point = 3;
     function empowerArrows(scope) {
         $('.poll-move-up', scope).click(function () {
-            var tag = $(this).parent().parent().parent();
+            var tag = $(this).parents('li');
             if (tag.index() <= starting_point){
                 return;
             }
             tag.prev().before(tag);
-            tag.fadeOut(250).fadeIn(250);
+            tag.fadeOut("fast", "swing").fadeIn("fast", "swing");
         });
         $('.poll-move-down', scope).click(function () {
-            var tag = $(this).parent().parent().parent();
+            var tag = $(this).parents('li');
             if ((tag.index() >= (tag.parent().children().length - 1))) {
                 return;
             }
             tag.next().after(tag);
-            tag.parent().parent().parent().scrollTop(tag.offset().top);
-            tag.fadeOut(250).fadeIn(250);
+            tag.fadeOut("fast", "swing").fadeIn("fast", "swing");
         });
     }
 
@@ -50,13 +43,23 @@ function PollEditBlock(runtime, element) {
     }
 
     $('#poll-add-answer', element).click(function () {
-        pollLineItems.append(answerTemplate({'answers': [{'key': generateUUID(), 'text': ''}]}));
+        // The degree of precision on date should be precise enough to avoid
+        // collisions in the real world.
+        pollLineItems.append(answerTemplate({'answers': [{'key': new Date().getTime(), 'text': ''}]}));
         var new_answer = $(pollLineItems.children().last());
         empowerDeletes(new_answer);
         empowerArrows(new_answer);
         new_answer.fadeOut(250).fadeIn(250);
     });
 
+    var to_disable = ['#poll-add-answer-link', 'input[type=submit', '.poll-delete-answer'];
+    for (var selector in to_disable) {
+        $(selector, element).click(function(event) {
+                event.preventDefault();
+            }
+        )
+    }
+
     $(element).find('.cancel-button', element).bind('click', function() {
         runtime.notify('cancel', {});
     });

tests/integration/base_test.py

@@ -3,4 +3,7 @@ from xblockutils.base_test import SeleniumBaseTest
 
 class PollBaseTest(SeleniumBaseTest):
     default_css_selector = 'div.poll-block'
-    module_name = __name__
+    module_name = __name__
\ No newline at end of file
+
+    def get_submit(self):
+        return self.browser.find_element_by_css_selector('input[name="poll-submit"]')
\ No newline at end of file

tests/integration/test_defaults.py

@@ -20,9 +20,11 @@ class TestDefaults(PollBaseTest):
         self.go_to_page('Defaults')
         button = self.browser.find_element_by_css_selector('input[type=radio]')
         button.click()
-        submit = self.browser.find_element_by_css_selector('input[name="poll-submit"]')
+        submit = self.get_submit()
         submit.click()
 
+        self.wait_until_exists('.poll-percent-display')
+
         # Should now be on the results page.
         self.assertEqual(self.browser.find_element_by_css_selector('.poll-percent-display').text, '100%')
 

tests/integration/test_layout.py

@@ -22,9 +22,9 @@ class TestLayout(PollBaseTest):
 
         # Pics should be within labels.
         pics[0].find_element_by_css_selector('img').click()
-        self.browser.find_element_by_css_selector('input[name=poll-submit]').click()
+        self.get_submit().click()
 
-        time.sleep(1)
+        self.wait_until_exists('.poll-image')
 
         self.assertEqual(len(self.browser.find_elements_by_css_selector('.poll-image')), 4)
 
@@ -39,8 +39,8 @@ class TestLayout(PollBaseTest):
 
         pics[0].find_element_by_css_selector('img').click()
 
-        self.browser.find_element_by_css_selector('input[name=poll-submit]').click()
+        self.get_submit().click()
 
-        time.sleep(1)
+        self.wait_until_exists('.poll-image.result-image')
         # ...But on the results page, we need four, for table layout.
         self.assertEqual(len(self.browser.find_elements_by_css_selector('.poll-image')), 4)
\ No newline at end of file

tests/integration/test_markdown.py

@@ -32,8 +32,9 @@ We shall find out if markdown is respected.
         """
         self.go_to_page("Markdown")
         self.browser.find_element_by_css_selector('input[type=radio]').click()
-        self.browser.find_element_by_css_selector('input[name="poll-submit"]').click()
+        self.get_submit().click()
 
+        self.wait_until_exists('.poll-feedback')
         self.assertEqual(
             self.browser.find_element_by_css_selector('.poll-feedback').text,
             """This is some feedback

tests/integration/test_poll_functions.py

@@ -13,8 +13,7 @@ class TestPollFunctions(PollBaseTest):
         Checks first load.
 
         Verify that the poll loads with the expected choices, that feedback is
-        not showing, that the submit button is disabled, and that it is enabled
-        when a choice is selected.
+        not showing, and that the submit button is disabled.
         """
         self.go_to_page('Poll Functions')
         answer_elements = self.browser.find_elements_by_css_selector('label.poll-answer')
@@ -23,13 +22,19 @@ class TestPollFunctions(PollBaseTest):
 
         self.assertRaises(NoSuchElementException, self.browser.find_element_by_css_selector, '.poll-feedback')
 
-        submit_button = self.browser.find_element_by_css_selector('input[name=poll-submit]')
+        submit_button = self.get_submit()
         self.assertFalse(submit_button.is_enabled())
 
+    def test_submit_enabled(self):
+        """
+        Makes sure the submit button is enabled when selecting an answer.
+        """
+        self.go_to_page('Poll Functions')
+        answer_elements = self.browser.find_elements_by_css_selector('label.poll-answer')
         answer_elements[0].click()
 
         # When an answer is selected, make sure submit is enabled.
-        self.assertTrue(submit_button.is_enabled())
+        self.wait_until_exists('input[name=poll-submit]:enabled')
 
     def test_poll_submission(self):
         """
@@ -43,7 +48,7 @@ class TestPollFunctions(PollBaseTest):
         # 'Not very long'
         answer_elements[1].click()
 
-        self.browser.find_element_by_css_selector('input[name=poll-submit]').click()
+        self.get_submit().click()
 
         # Not a good way to wait here, since all the elements we care about
         # tracking don't exist yet.
@@ -68,12 +73,10 @@ class TestPollFunctions(PollBaseTest):
         # Not very long
         answer_elements[1].click()
 
-        self.browser.find_element_by_css_selector('input[name=poll-submit]').click()
+        self.get_submit().click()
 
-        time.sleep(1)
-
-        submit_button = self.browser.find_element_by_css_selector('input[name=poll-submit]')
-        self.assertFalse(submit_button.is_enabled())
+        # Button will be reaplaced with a new disabled copy, not just disabled.
+        self.wait_until_exists('input[name=poll-submit]:disabled')
 
         self.go_to_page('Poll Functions')
-        self.assertFalse(self.browser.find_element_by_css_selector('input[name=poll-submit]').is_enabled())
+        self.assertFalse(self.get_submit().is_enabled())
\ No newline at end of file