Convert the review request field changes to modern CSS components.

Review Request #13907 — Created May 29, 2024 and submitted

Information

Review Board
release-7.x

Reviewers

The lists of changed fields on a "review request changed" box was
ancient, with spacing and font size inconsistencies, mobile issues, no
accessibility affordances, and some regressions with recent styling
changes.

This has been redone, as a sort of first pass that we can build upon in
the future. We now have two new components we can use:

  • rb-c-review-request-changed-fields: The container for the entire
    list of changed fields in the entry box, managing overall layout and
    the styling for inner value types.

  • rb-c-review-request-changed-value: An old/new diff display for a
    single simple field value, such as Branch, Summary, or Depends On.

The rb-c-review-request-changed-fields component is a definition list
consisting of field names and their value changes. It uses CSS grid to
do the layout, in a responsive manner. Subgrids are employed when
supported by the browser in order to help all fields line up correctly
(without it, we use a minimum width to achieve a similar effect).

There's now one of these lists instead of many, which can help with
navigating using assistive technologies. We also make use of ARIA labels
throughout this.

rb-c-review-request-changed-value works as the old field changes did
before, but is slightly polished in that it's now less boxy, employing a
soft rounded border.

The changed fields component optimistically aligns the labels and values
with a baseline. This works great for the changed value component and
other simple text-based value representations. However, in an effort to
properly align things, special rules for certain value components are
added. This gives us alignment for diffed text displays, file attachment
thumbnails, and the commits list.

Field changes were made to aid in this and to help with consistency:

  • File attachments now have a parent container that we can set to be a
    flexbox.
  • Status now says "Completed" instead of "Closed (submitted)".

Some fields (namely file attachment captions) still need a lot of love,
but that's for another release.

The overall presentation, while resembling what we had before, is now
much more polished, and will be easier to work with going forward.

Tested in Chrome and Firefox in light and dark modes.

Tested with and without subgrid support (by altering the name checked
by @support), and on mobile. Verified the presentation was correct
in each case.

Tested every field type in all the above cases.

Summary ID
Convert the review request field changes to modern CSS components.
The lists of changed fields on a "review request changed" box was ancient, with spacing and font size inconsistencies, mobile issues, no accessibility affordances, and some regressions with recent styling changes. This has been redone, as a sort of first pass that we can build upon in the future. We now have two new components we can use: * `rb-c-review-request-changed-fields`: The container for the entire list of changed fields in the entry box, managing overall layout and the styling for inner value types. * `rb-c-review-request-changed-value`: An old/new diff display for a single simple field value, such as Branch, Summary, or Depends On. The `rb-c-review-request-changed-fields` component is a definition list consisting of field names and their value changes. It uses CSS grid to do the layout, in a responsive manner. Subgrids are employed when supported by the browser in order to help all fields line up correctly (without it, we use a minimum width to achieve a similar effect). There's now one of these lists instead of many, which can help with navigating using assistive technologies. We also make use of ARIA labels throughout this. `rb-c-review-request-changed-value` works as the old field changes did before, but is slightly polished in that it's now less boxy, employing a soft rounded border. The changed fields component optimistically aligns the labels and values with a baseline. This works great for the changed value component and other simple text-based value representations. However, in an effort to properly align things, special rules for certain value components are added. This gives us alignment for diffed text displays, file attachment thumbnails, and the commits list. The overall presentation, while resembling what we had before, is now much more polished, and will be easier to work with going forward.
abd231556e3d46370a6650a788445e01cf5fa463

Description From Last Updated

Leftover code?

maubinmaubin

Is this still needed even though we have flush-codehilite();? If so could you change the comment from // to /* …

maubinmaubin
maubin
  1. 
      
  2. Show all issues

    Leftover code?

  3. Show all issues

    Is this still needed even though we have flush-codehilite();? If so could you change the comment from // to /* .. */ for consistency.

    1. Yeah, flush-codehilite doesn't seem to work in all cases. We have to override things in a couple places. This is all copy/paste code from diffviewer.less.

  4. 
      
chipx86
maubin
  1. Ship It!
  2. 
      
david
  1. Ship It!
  2. 
      
chipx86
Review request changed
Status:
Completed
Change Summary:
Pushed to release-7.x (e481938)