Modified styling for diff viewer lines to be colorblind-friendly
Review Request #10981 — Created March 29, 2020 and updated
Information | |
---|---|
kpatenio | |
Review Board | |
release-4.0.x | |
87859c5... | |
Reviewers | |
reviewboard | |
The red, green, and yellow colors used in Review Board's diff viewer aren't
easily dintinguishable for users with various colorblind disabilities.To address this, icons have been added to inserted, replaced, and deleted
diff viewer lines (+, ~, and - respectively) to visually represent the type
of change made without having to solely rely on color. Alignment for equal
lines (unmodified lines of code) was also updated; line numbers for all
diff types should now be aligned.I've attached a video below showing how the diff viewer would like with my
new changes.For more information on how I came up with these changes, see my notes here.
All current unit tests pass. No additional tests were made.
Description | From | Last Updated |
---|---|---|
Could we perhaps reuse some of the builtin colors already defined in reviewboard/reviewboard/static/rb/css/ui/colors.less? For example, #rb-ns-ui.colors[@red-60]; You'll just need to … |
![]() |
|
Same comment here about reusing colors #rb-ns-ui.colors[@green-70]; |
![]() |
|
Same comment here about colors #rb-ns-ui.colors[@yellow-90];. I know the shades of yellow here are limited so if they don't work … |
![]() |
|
I think this comment should use /* ... */ |
|

Change Summary:
Make a .less mixin to be used by all diff lines + uploaded more screenshots
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 2 (+20) |
||||
Added Files: |
Checks run (2 succeeded)

-
-
reviewboard/static/rb/css/pages/diffviewer.less (Diff revision 2) Could we perhaps reuse some of the builtin colors already defined in
reviewboard/reviewboard/static/rb/css/ui/colors.less
?For example,
#rb-ns-ui.colors[@red-60];
You'll just need to play around with the different shades of red to get the best one you need for the job
-
reviewboard/static/rb/css/pages/diffviewer.less (Diff revision 2) Same comment here about reusing colors
#rb-ns-ui.colors[@green-70];
-
reviewboard/static/rb/css/pages/diffviewer.less (Diff revision 2) Same comment here about colors
#rb-ns-ui.colors[@yellow-90];
.I know the shades of yellow here are limited so if they don't work out, I think leaving it as is should ok or even defining it as a variable for future reusability is also a good idea.

Change Summary:
Utilized
colors.less
mixin for colour values
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 3 (+25 -1) |
Checks run (2 succeeded)
-
This looks pretty good! One another suggestion is to add a short comment of what the attached screenshots are for and possibly updating their captions. For a small change like this, it's relatively obvious that they're showing the UI changes but in a review request that might have multiple attached files to explain different parts of the change, it might not be so clear.
-
reviewboard/static/rb/css/pages/diffviewer.less (Diff revision 3) I think this comment should use
/* ... */

Change Summary:
Updated description to be more clear + fixed comment styling
Description: |
|
|||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Commit: |
|
|||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 4 (+27 -1) |
-
File Captions:
Screen Shot 2020-03-29 at 16.10.57.png: - Screen Shot 2020-03-29 at 16.10.57.png + Multiline "modified" (yellow) diff changes. Screen Shot 2020-03-29 at 16.11.19.png: - Screen Shot 2020-03-29 at 16.11.19.png + Multiline "insert" (green) and "deleted" (red) diff changes.
Checks run (2 succeeded)

Change Summary:
Updated styling to be consistent + more aethestically pleasing
Description: |
|
|||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Commit: |
|
|||||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 5 (+42) |
|||||||||||||||||||||||||||||||||||||||||||||
Removed Files: |
||||||||||||||||||||||||||||||||||||||||||||||
Added Files: |
Checks run (2 succeeded)

Change Summary:
Fixed alignment issue for line numbers of equal diffs + rebased with release 4
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 6 (+44) |
Checks run (2 succeeded)

Change Summary:
Updated description
Description: |
|
---|