From adfca9d4801d5ad35ca52474d3ee0d89e1afb2e4 Mon Sep 17 00:00:00 2001 From: Awais Jibran Date: Mon, 25 May 2015 16:39:53 +0500 Subject: [PATCH] TNL-2223 Inline discussion jumps to the bottom of the page when thread is expanded. --- .../views/discussion_thread_view.coffee | 9 +++-- common/test/acceptance/fixtures/discussion.py | 14 ++++++++ .../test/acceptance/pages/lms/discussion.py | 7 ++++ .../tests/discussion/test_discussion.py | 33 +++++++++++++++++++ 4 files changed, 61 insertions(+), 2 deletions(-) diff --git a/common/static/coffee/src/discussion/views/discussion_thread_view.coffee b/common/static/coffee/src/discussion/views/discussion_thread_view.coffee index 69d45e1acc..ce893c38e7 100644 --- a/common/static/coffee/src/discussion/views/discussion_thread_view.coffee +++ b/common/static/coffee/src/discussion/views/discussion_thread_view.coffee @@ -118,6 +118,12 @@ if Backbone? @responsesRequest.abort() loadResponses: (responseLimit, elem, firstLoad) -> + # takeFocus take the page focus to response loading element while responses are being fetched. + # - When viewing in the Discussions tab, responses are loaded automatically, Do not scroll to the + # element(TNL-1530) + # - When viewing inline in courseware, user clicks 'expand' to open responses, Its ok to scroll to the + # element (Default) + takeFocus = if @mode == "tab" then false else true @responsesRequest = DiscussionUtil.safeAjax url: DiscussionUtil.urlFor('retrieve_single_thread', @model.get('commentable_id'), @model.id) data: @@ -125,7 +131,7 @@ if Backbone? resp_limit: responseLimit if responseLimit $elem: elem $loading: elem - takeFocus: false + takeFocus: takeFocus complete: => @responsesRequest = null success: (data, textStatus, xhr) => @@ -144,7 +150,6 @@ if Backbone? ) @trigger "thread:responses:rendered" @loadedResponses = true - $(".thread-wrapper").focus() # Sends focus to the conversation once the thread finishes loading error: (xhr, textStatus) => return if textStatus == 'abort' diff --git a/common/test/acceptance/fixtures/discussion.py b/common/test/acceptance/fixtures/discussion.py index 38b20c768b..b99ba94f23 100644 --- a/common/test/acceptance/fixtures/discussion.py +++ b/common/test/acceptance/fixtures/discussion.py @@ -135,6 +135,20 @@ class MultipleThreadFixture(DiscussionContentFixture): threads_list = {thread['id']: thread for thread in self.threads} return {"threads": json.dumps(threads_list), "comments": '{}'} + def add_response(self, response, comments, thread): + """ + Add responses to the thread + """ + response['children'] = comments + if thread["thread_type"] == "discussion": + response_list_attr = "children" + elif response["endorsed"]: + response_list_attr = "endorsed_responses" + else: + response_list_attr = "non_endorsed_responses" + thread.setdefault(response_list_attr, []).append(response) + thread['comments_count'] += len(comments) + 1 + class UserProfileViewFixture(DiscussionContentFixture): diff --git a/common/test/acceptance/pages/lms/discussion.py b/common/test/acceptance/pages/lms/discussion.py index 6bcfb85433..3897738956 100644 --- a/common/test/acceptance/pages/lms/discussion.py +++ b/common/test/acceptance/pages/lms/discussion.py @@ -464,6 +464,13 @@ class InlineDiscussionThreadPage(DiscussionThreadPage): def is_thread_anonymous(self): return not self.q(css=".posted-details > .username").present + @wait_for_js + def check_if_selector_is_focused(self, selector): + """ + Check if selector is focused + """ + return self.browser.execute_script("return $('{}').is(':focus')".format(selector)) + class DiscussionUserProfilePage(CoursePage): diff --git a/common/test/acceptance/tests/discussion/test_discussion.py b/common/test/acceptance/tests/discussion/test_discussion.py index e5d8ef45a1..f213038b0d 100644 --- a/common/test/acceptance/tests/discussion/test_discussion.py +++ b/common/test/acceptance/tests/discussion/test_discussion.py @@ -582,6 +582,7 @@ class InlineDiscussionTest(UniqueCourseTest, DiscussionResponsePaginationTestMix def setUp(self): super(InlineDiscussionTest, self).setUp() + self.thread_ids = [] self.discussion_id = "test_discussion_{}".format(uuid4().hex) self.additional_discussion_id = "test_discussion_{}".format(uuid4().hex) self.course_fix = CourseFixture(**self.course_info).add_children( @@ -616,6 +617,38 @@ class InlineDiscussionTest(UniqueCourseTest, DiscussionResponsePaginationTestMix self.thread_page = InlineDiscussionThreadPage(self.browser, thread_id) # pylint: disable=attribute-defined-outside-init self.thread_page.expand() + def setup_multiple_inline_threads(self, thread_count): + """ + Set up multiple treads on the page by passing 'thread_count' + """ + threads = [] + for i in range(thread_count): + thread_id = "test_thread_{}_{}".format(i, uuid4().hex) + threads.append( + Thread(id=thread_id, commentable_id=self.discussion_id), + ) + self.thread_ids.append(thread_id) + thread_fixture = MultipleThreadFixture(threads) + thread_fixture.add_response( + Response(id="response1"), + [Comment(id="comment1", user_id="other"), Comment(id="comment2", user_id=self.user_id)], + threads[0] + ) + thread_fixture.push() + + def test_page_while_expanding_inline_discussion(self): + """ + Tests for the Inline Discussion page with multiple treads. Page should not focus 'thread-wrapper' + after loading responses. + """ + self.setup_multiple_inline_threads(thread_count=3) + self.discussion_page.expand_discussion() + thread_page = InlineDiscussionThreadPage(self.browser, self.thread_ids[0]) + thread_page.expand() + + # Check if 'thread-wrapper' is focused after expanding thread + self.assertFalse(thread_page.check_if_selector_is_focused(selector='.thread-wrapper')) + def test_initial_render(self): self.assertFalse(self.discussion_page.is_discussion_expanded())