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	ID
Disentangle styles for review request tabs and improve tab layout. 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.	bc749e3bc053c9f6b4f86d55b1a2b8274d823522

Summary

Disentangle styles for review request tabs and improve tab layout.

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.

bc749e3bc053c9f6b4f86d55b1a2b8274d823522

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	ID
	Disentangle styles for review request tabs and actions. 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. Testing Done: Looked at and interacted with tabs in Firefox, Chrome, and Safari.	af918ca3337d45321c415f1ecebf45a3ec61c8e1
	Disentangle styles for review request tabs and improve tab layout. 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.	6aa5c9e25b8dfc1020dbd65d63b9ce046cd0a942

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).
```