Summary:

Disentangle styles for review request tabs and improve tab layout.

Review Request #12749 — Created Dec. 1, 2022 and submitted Feb. 26, 2023, 3:32 p.m.

Information
Owner:	david
Repository:	Review Board
Branch:	release-6.x
Bugs:
Depends On:
Reviewers
Groups:	reviewboard
People:

Description

The review request tabs element was also adding the review-request-actions
class, picking up a whole bunch of style rules. Most of these rules were
not necessary, either being specific to actions, or overridden by the
tabs.

This change splits those up, and also redoes the tab layout using our
new CSS component style. With this, the layout and CSS use our modern
standards, and use the more-semantic <menu> element. I've also added
ARIA attributes where appropriate to ensure that these elements include
the proper roles and attributes.

Testing Done


Looked at and interacted with reviews, diff, and file attachment tabs in

  Firefox, Chrome, and Safari. Verified that functionality and

  appearance are correct.
Used a11y debug tools to validate the layout and roles of the tabs.

Commits

Summary
	Disentangle styles for review request tabs and improve tab layout.

Screenshots

Issues

Description	From	Last Updated
Can you also add documentation for each named part of the component?	chipx86	Jan. 10, 2023, 8:10 a.m.
Indentation got a bit wonky here.	chipx86	Feb. 3, 2023, 9:41 p.m.
The & rules should go above rules for tags (a).	chipx86	Dec. 22, 2022, 9:14 a.m.
These should be in alphabetical order.	chipx86	Feb. 26, 2023, 3:29 p.m.

flake8 passed.

JSHint passed.

Summary:

-	Disentangle styles for review request tabs and actions.
+	Disentangle styles for review request tabs and improve tab layout.

Description:

		The review request tabs element was also adding the `review-request-actions`
~		class, picking up a whole bunch of style rules. It turns out that only a
~		couple of these rules were not already included in the tabs CSS, so I've
~		copied them there and removed the actions class.
	~	class, picking up a whole bunch of style rules. Most of these rules were
	~	not necessary, either being specific to actions, or overridden by the
	~	tabs.
	+
	+	This change splits those up, and also redoes the tab layout using our
	+	new CSS component style. With this, the layout and CSS use our modern
	+	standards, and use the more-semantic `<menu>` element. I've also added
	+	ARIA attributes where appropriate to ensure that these elements include
	+	the proper roles and attributes.

Testing Done:

~		Looked at and interacted with tabs in Firefox, Chrome, and Safari.
	~	Looked at and interacted with reviews, diff, and file attachment tabs in Firefox, Chrome, and Safari. Verified that functionality and appearance are correct.
	+	Used a11y debug tools to validate the layout and roles of the tabs.

Commits:

	Summary
-		Disentangle styles for review request tabs and actions.
+		Disentangle styles for review request tabs and improve tab layout.

Diff:

Revision 2 (+134 -32)

Show changes

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

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

Side note: What a11y debug tools do you use? I've never used any before but this seems useful and accessibility is something I've been wanting to be more mindful of.

david Dec. 2, 2022, 11:46 a.m.

Firefox has an "Accessibility" panel in the devtools which I use for most of it. Chrome has some similar tools, though they're a bit more scattered through their devtools UI.

You can also enable a screen reader for your OS and try to tab around with your eyes closed if you want to be humbled :)

maubin Dec. 2, 2022, 12:05 p.m.
```
Heh cool good to know.
```

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

Show all issues

Can you also add documentation for each named part of the component?

david Dec. 22, 2022, 9:14 a.m.
```
Can you explain what you mean by this?
```

reviewboard/static/rb/css/pages/review-request.less (Diff revision 2)
Show all issues
```
The & rules should go above rules for tags (a).
```