Summary:

Change tab index on review request page (student project)

Review Request #10848 — Created Jan. 22, 2020 and updated Feb. 22, 2020, 4:13 p.m.

Information
Owner:	LittleGrey
Repository:	Review Board
Branch:	master
Bugs:
Depends On:
Reviewers
Groups:	reviewboard
People:

Description

The review request page has a weird keyboard behavior. When you
want to tab-navigate, the ordering is strange. Hitting tab
repeatedly will get you to the right-hand column
(review-request-details), and then to the left-hand
(request-request-main) column, which seems backwards, and
it takes a while to even get there.

I switched the HTML code blocks of these two elements so that
hitting tab will get you to left column -> right column. Also
I modified a few CSS lines to avoid element arrangement
problems (the two columns are not at the same height if I
didn't change the CSS).

A demo video is at here.

Testing Done

Click on the top fields, hitting tabs until it gets to the left column and then right column.

Commits

Summary	Author
Change tab index on review request page	Xiaohui Liu
Order CSS properties alphabetically	Xiaohui Liu
Make changes compatilble with mobile screens	Jay

Screenshots

Files

Issues

Description	From	Last Updated
Can you show screenshots of this in both mobile and desktop modes? It's important that mobile doesn't break. You can …	chipx86	Feb. 23, 2020, 3:10 p.m.

flake8 passed.

JSHint passed.

Description:

		The review request page has a weird keyboard behavior. When you
		want to tab-navigate, the ordering is strange. Hitting tab
		repeatedly will get you to the right-hand column
		(`review-request-details`), and then to the left-hand
		(`request-request-main`) column, which seems backwards, and
		it takes a while to even get there.

		I switched the HTML code blocks of these two elements so that
		hitting tab will get you to left column -> right column. Also
		I modified a few CSS lines to avoid element arrangement
		problems (the two columns are not at the same height if I
		didn't change the CSS).
	+
	+	A demo video is at here.

reviewboard/static/rb/css/pages/review-request.less (Diff revision 1)

According to the style from other blocks, my suggestion could be swithing the order of width and position in alphabetial order.

It doesn't mention in css/less style guide, so I am not sure it will be a correct coding style.

LittleGrey Jan. 23, 2020, 1:50 p.m.

Good point! I'll confirm this with the mentors.

Commits:

	Summary
-	Change tab index on review request page
+	Change tab index on review request page
+	Order CSS properties alphabetically

Diff:

Revision 2 (+27 -25)

Show changes

	reviewboard/static/rb/css/pages/review-request.less
	reviewboard/templates/reviews/review_request_box.html

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Show all issues

Can you show screenshots of this in both mobile and desktop modes? It's important that mobile doesn't break.

You can use the browser's responsive viewing mode to test this.

LittleGrey Feb. 22, 2020, 4:13 p.m.

Sorry for the delay. Just uploaded the screenshots. Had been blocked by the CSS tricks quite a while :(

Change Summary:

Make the layout mobile-friendly.

Commits:

	Summary	Author
-	Change tab index on review request page	Xiaohui Liu
-	Order CSS properties alphabetically	Xiaohui Liu
+	Change tab index on review request page	Xiaohui Liu
+	Order CSS properties alphabetically	Xiaohui Liu
+	Make changes compatilble with mobile screens	Jay

Diff:

Revision 3 (+30 -30)

Show changes

	reviewboard/static/rb/css/pages/review-request.less
	reviewboard/templates/reviews/review_request_box.html

Added Files:

Checks run (2 succeeded)

flake8 passed.

JSHint passed.