Disentangle styles for review request tabs and improve tab layout.

Review Request #12749 — Created Dec. 1, 2022 and submitted

Information

Review Board
release-6.x

Reviewers

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.

  • 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.
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
Description From Last Updated

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

chipx86chipx86

Indentation got a bit wonky here.

chipx86chipx86

The & rules should go above rules for tags (a).

chipx86chipx86

These should be in alphabetical order.

chipx86chipx86
david
maubin
  1. 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.

    1. 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 :)

    2. Heh cool good to know.

  2. 
      
chipx86
  1. 
      
  2. Show all issues

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

    1. Can you explain what you mean by this?

  3. Show all issues

    The & rules should go above rules for tags (a).

  4. 
      
david
chipx86
  1. 
      
  2. reviewboard/static/rb/css/pages/review-request.less (Diff revisions 2 - 3)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
    Show all issues

    Indentation got a bit wonky here.

  3. 
      
david
chipx86
  1. One tiny thing.

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

    These should be in alphabetical order.

  3. 
      
david
Review request changed
Status:
Completed
Change Summary:
Pushed to release-6.x (50f24ad)