flake8
passed.
JSHint
passed.
Modified styling for diff viewer lines to be colorblind-friendly
Review Request #10981 — Created March 29, 2020 and updated
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 … |
 bui1
|
|
|
Same comment here about reusing colors #rb-ns-ui.colors[@green-70]; |
bui1
|
|
|
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 … |
bui1
|
|
|
I think this comment should use /* ... */ |
 hxqlin
|
- Change Summary:
-
Make a .less mixin to be used by all diff lines + uploaded more screenshots
- Commit:
-
1c4e4451550a2ea7b32f01a6b02ddc8500c4081d283619d65c6ba12bcfc37ef6165a411823652539
- Added Files:
Checks run (2 succeeded)
flake8
passed.
JSHint
passed.
-
-
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
-
-
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.lessmixin for colour values - Commit:
-
283619d65c6ba12bcfc37ef6165a4118236525391fa7fa806e31f61886762b0a4f6c3b0b9923d001
Checks run (2 succeeded)
flake8
passed.
JSHint
passed.
-
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.
-
- Change Summary:
-
Updated description to be more clear + fixed comment styling
- Description:
-
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. + For a visual representation of these changes, I've attached screenshots
+ below. I chose screenshots showing the three different types of diffs, + both as multi-line changes and as single line changes, in order to + demonstrate the readability of the diffs with the newly added icons. + For more information, see my notes here.
- Commit:
-
1fa7fa806e31f61886762b0a4f6c3b0b9923d001d0dd941b463f8e75538c94638648c4eac10ef295
- 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.pngMultiline "modified" (yellow) diff changes.Screen Shot 2020-03-29 at 16.11.19.png: Screen Shot 2020-03-29 at 16.11.19.pngMultiline "insert" (green) and "deleted" (red) diff changes.
Checks run (2 succeeded)
flake8
passed.
JSHint
passed.
- Change Summary:
-
Updated styling to be consistent + more aethestically pleasing
- Description:
-
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. For a visual representation of these changes, I've attached screenshots
~ below. I chose screenshots showing the three different types of diffs, ~ both as multi-line changes and as single line changes, in order to ~ below. I chose screenshots showing the three different types of diffs; this ~ is to demonstrate the readability of the diffs with the newly added icons. - demonstrate the readability of the diffs with the newly added icons. For more information, see my notes here.
- Commit:
-
d0dd941b463f8e75538c94638648c4eac10ef295855ea882c2535ad4de4470fe3523d61df2f78267
- Diff:
-
Revision 5 (+42)
- Removed Files:
- Added Files:
Checks run (2 succeeded)
flake8
passed.
JSHint
passed.
- Change Summary:
-
Fixed alignment issue for line numbers of equal diffs + rebased with release 4
- Commit:
-
855ea882c2535ad4de4470fe3523d61df2f7826787859c54896e9ca65be247161f8cf3657b9521e8
Checks run (2 succeeded)
flake8
passed.
JSHint
passed.
- Change Summary:
-
Updated description
- Description:
-
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. ~ 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. ~ For a visual representation of these changes, I've attached screenshots
~ below. I chose screenshots showing the three different types of diffs; this ~ I've attached a video below showing how the diff viewer would like with my
~ new changes. - is to demonstrate the readability of the diffs with the newly added icons. ~ For more information, see my notes here.
~ For more information on how I came up with these changes, see my notes here.