Skip to content

Improvements in forums nav sidebar #1169

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
gwprice merged 2 commits into from
Sep 27, 2013
Merged

Improvements in forums nav sidebar #1169

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f39886e
Ensure nav sidebar is sized correctly on page load
Sep 27, 2013
cbbb5c7
Improve focus management of forums nav sidebar
Sep 27, 2013
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
13 changes: 13 additions & 0 deletions common/static/coffee/src/discussion/views/discussion_thread_list_view.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ if Backbone?
@timer = 0
@$el.html(@template())

$(window).bind "load", @updateSidebar
$(window).bind "scroll", @updateSidebar
$(window).bind "resize", @updateSidebar

Expand Down Expand Up @@ -143,6 +144,18 @@ if Backbone?
options.group_id = @group_id


lastThread = @collection.last()?.get('id')

@kevinchugh kevinchugh Sep 27, 2013

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.

can you conditionally run this if there is no focus? because this is saying always refocus the last thread, but isnt' there a case where the user is on the second from last thread for example, and then loads more, and those threads append to the end? in this case, don't you want to not change the focus?

@gwprice gwprice Sep 27, 2013

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This is focusing the first thread that follows what was previously the last thread. It's not possible for the user to be focused on the second-to-last thread, because interacting with the "Load more" link moves focus there. One could argue that you don't want to move focus when you do something like click a sort button, but I think that always moving focus is the right thing to do.

@kevinchugh kevinchugh Sep 27, 2013

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.

oh right right. not to split hairs, and i'm fine with ignore this, but you can tab to the load more while keeping another thread focused and click the load more link with a screen reader. i think it's acceptable behavior to use your algorithm however.

if lastThread
# Pagination; focus the first thread after what was previously the last thread
@once("threads:rendered", ->
$(".post-list li:has(a[data-id='#{lastThread}']) + li a").focus()
)
else
# Totally refreshing the list (e.g. from clicking a sort button); focus the first thread
@once("threads:rendered", ->
$(".post-list a").first()?.focus()
)

@collection.retrieveAnotherPage(@mode, options, {sort_key: @sortBy})

renderThread: (thread) =>
Expand Down