Improve styling and accessibility of the diff collapse button.

Review Request #13230 — Created Aug. 23, 2023 and submitted

Information

Review Board
release-6.x

Reviewers

This gives the diff collapse button a fresher look, helping it to fit in
with the new diff viewer styles. It's now the same shade of blue as the
diff headers, has a slightly rounded look, and says "Collapse" rather
than just showing an icon.

This button has been converted to a CSS component, and the HTML in the
templates de-duplicated, helping to keep this all manageable. It's also
been given ARIA attributes and a role to help screen readers (although
there are still usability challenges with the DOM placement of this
button). Further improvement for specifying line numbers and the
filename for screen readers would be helpful as well, but is a larger
task.

The z-index of the button has been fixed so it won't float above the
Unified Banner.

Some of the old CSS around this button have been cleaned up.

The button icon is handled by mixing in a .rb-icon-... definition.
This required partially reverting a recent change to make all
definitions private in LessCSS. The reason for that issue before was
that two mixins were named .icon(), and thus were combining into one
rule. Since both have been renamed, this is no longer a problem. To
further help avoid issues, .rb-icon() has been renamed to
.make-rb-icon().

Unit tests pass.

Tested the collapse button in the diff viewer in various kinds of
expanded content, and in diff fragments for changed, added, and
deleted files. Verified positioning and behavior in each.

Verified that there was no issues between datagrid and Review Board
icons.

Summary ID
Improve styling and accessibility of the diff collapse button.
This gives the diff collapse button a fresher look, helping it to fit in with the new diff viewer styles. It's now the same shade of blue as the diff headers, has a slightly rounded look, and says "Collapse" rather than just showing an icon. This button has been converted to a CSS component, and the HTML in the templates de-duplicated, helping to keep this all manageable. It's also been given ARIA attributes and a role to help screen readers (although there are still usability challenges with the DOM placement of this button). Further improvement for specifying line numbers and the filename for screen readers would be helpful as well, but is a larger task. Some of the old CSS around this button have been cleaned up. The button icon is handled by mixing in a `.rb-icon-...` definition. This required partially reverting a recent change to make all definitions private in LessCSS. The reason for that issue before was that two mixins were named `.icon()`, and thus were combining into one rule. Since both have been renamed, this is no longer a problem. To further help avoid issues, `.rb-icon()` has been renamed to `.make-rb-icon()`.
977bf9c7d5a417967403e2cb17f91d5863fb57d3
maubin
  1. Can you add a picture of the new button?

    1. Nvm, I see it in your other change.

  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-6.x (ffb0e89)
david
  1. Ship It!
  2. 
      
Loading...