JSHint
-
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 1) Show all issues
Review Request #11445 — Created Feb. 14, 2021 and updated
This will allow users to select the colour outside of the image to help
improve visibility. For instance if an image is mostlywhite and the background
is white it may be difficult tounderstand what the image is. The background
colour selector menu is present on all image review modes (i.e. single
revision, "no diff", two-up, difference, split, and onion skin). The
background colour selector and the image resolution menu have now been moved
to a line below the caption in the image reviewer in order to better control
the arrangements of all elements in the header when the screen is adjusted to
a smaller size.
Manual testing has been done to ensure that the background color of the image
review page can change to white, black, grey/black checkerboard, or a custom
colour for all image views (i.e. single revision, "no diff", two-up, difference,
split, and onion skin). This has been tested on Mozilla Firefox, Safari and
Google Chrome and the menu appears as intended.
Summary | ID | Author |
---|---|---|
f7188254230e300284e8085f30c177bda2664905 | mderose123 | |
a4f46881c26a25a75417cf57716c36e172bc010c | mderose123 | |
d1d129a1d3f5b4c780331a29f890cb1dc22728ee | mderose123 | |
583727ca00da0095be8b3b244a08c68a9ecf2921 | mderose123 | |
8c99f19399f2b0b7dfb94bb4c959c8db5c72523c | mderose123 | |
9a0dd0e977d97ad7702ea5cc5eb517d803576304 | mderose123 | |
39fb276e93c847e4319821eb9d348cf549232563 | mderose123 | |
39a633b49bff00a1da86beac9691733449c19a14 | mderose123 | |
48fb7fe485a5fc74c7a3f8630b830fafaa8651ee | mderose123 | |
bee7caae3cecf3f88df5b88c36b75fef49e25c89 | mderose123 | |
2490fa8821291b5707bc1506cc6c3a215d4aa83b | mderose123 | |
33d6f50ceb97cb2b4d466be9060341070539936c | mderose123 | |
01fbb3543a9ea7c20d084f17bb6f5cd01547d0de | mderose123 | |
6c9124f6c8d6de2ae11845d280dd1fcab262ff3b | mderose123 | |
5ccdcdcba6ba5f627d88effdc1a8bdc71ecfe6d1 | mderose123 | |
ad5b3a07a8fedc817bfddbdd502c1cdcdeb0d2fc | mderose123 | |
2c98d7b0624a97dc11085a5a8b703bb5338bb64c | mderose123 | |
23b923a6bc741e31720e10ea44cef4d00af30d8c | mderose123 | |
e61b0278b85a765d2749ad505de93fa90db3503c | mderose123 | |
4f1dd30545cf9ae229884ec81b7efea762c465c4 | mderose123 |
Description | From | Last Updated |
---|---|---|
You're working with some older code that predates our modern CSS naming conventions, but it'd still be good to make … |
|
|
Please wrap your description and testing done sections to be 80 characters wide. |
|
|
I think, personally, this UI fits in really nicely with Review Board's pre-existing metaphors. Great job! A few notes: We … |
|
|
I think we should have the captions be last. We want to think of the UI in logical components. We … |
|
|
Col: 76 Missing semicolon. |
![]() |
|
Col: 97 Missing semicolon. |
![]() |
|
Col: 45 Missing semicolon. |
![]() |
|
Col: 67 Missing semicolon. |
![]() |
|
The text-align should predecede the nested table rule, because it's relevant to .review-ui-header. You'd also want a blank line between … |
|
|
Can you put this in alphabetical order? While not always appropriate for all CSS rules, it's a good point to … |
|
|
This is too generic a name, so it'll be especially important to namespace this, as per our docs. |
|
|
Blank line between these. |
|
|
Can you do testing on Firefox and Safari for these? |
|
|
Can you reorder these to be in alphabetical order? |
|
|
The blank line here should stay. |
|
|
Some important comments here: Keep line lengths in mind. Lines cannot go beyond 79 characters. These sorts of CSS changes … |
|
|
Blank line between these. |
|
|
Space after the if, before the (. |
|
|
Can you go through the file and make sure there's no extra whitespace (identified by these red blocks)? |
|
|
This would be best done as a _.template somewhere, using _.dedent (see other parts of the codebase for examples). |
|
|
Any text must be localized and provided in. This is easier done with templates. You can localize a string using: … |
|
|
Always use double quotes for attribute values, and never self-close tags. |
|
|
</> isn't valid. |
|
|
You can use chaining to reduce the code here: $('<div class="....">') .append($backgroundMenu) .append($resolutionMenu) .appendTo($header); |
|
|
You can set multiple model attributes at once: this.model.set({ checkeredBackground: false, background: color, }); This helps with ensuring that signal … |
|
|
This can be one statement. Also, code should always use single quotes instead of double quotes, unless the text in … |
|
|
Missing a period. This must also be one line (and can't go beyond the line length). |
|
|
This should probably be driven by the model. The model should listen for a change to checkeredBackground. If set, it … |
|
|
Col: 12 Unexpected ')'. |
![]() |
|
Col: 12 Expected an identifier and instead saw ')'. |
![]() |
|
Col: 13 Expected ')' and instead saw ';'. |
![]() |
|
Col: 14 Missing semicolon. |
![]() |
|
Col: 15 '$toolBar' is defined but never used. |
![]() |
|
Let's use #rb-ns-ui.colors[@grey-40] instead of #808080 |
|
|
This should also use one of the #rb-ns-ui.colors definitions (see reviewboard/static/rb/css/ui/colors.less) instead of "grey" |
|
|
Please use single quotes instead of double. |
|
|
Please use single quotes instead of double. |
|
|
This should probably be called backgroundMenuTemplate. The $ prefix is for jQuery objects. |
|
|
We could skip defining this as a variable and just do: $('<div class="...">') .append(...) |
|
|
Let's wrap this like so: .append(backgroundMenuTemplate({ backgroundLabel: backgroundLabel, customLabel: customLabel, }) .append($resolutionMenu) ... Note that we also don't need quotes … |
|
|
Please add a trailing comma at the end of this. |
|
|
I only see one place each where we use these. We don't need to define new variables for them. Let's … |
|
|
A more standardized name (in our modern standard) would be -has-checkered-background. |
|
|
Do we need !important? |
|
|
Blank line between these. |
|
|
We usually make display the first, or one of the first, properties in a rule. Can you move it up? |
|
|
Where possible, let's use em units. These are going to scale better than pixel units, making them more accessible for … |
|
|
No blank line here. |
|
|
These are looking pretty long. Not uncommon for this to happen. Try: events: { ... 'click .rb-o-image-background-blah-blah-blah': '_onBlahChanged', ... } |
|
|
When you're setting a single attribute, pass each in as a parameter: this.model.set('checkeredBackground', false); |
|
|
this.$el is the .image-review-ui component, so rather than scanning the entire DOM for it each time, you can just use … |
|
|
This can now be: this.$el.toggleClass('-has-checkered-background', isCheckeredBackground); |
|
|
All this should be 1-space indentation. |
|
|
Only one space indentation. This should also be <%- backgroundLabel %>, in case the text gets localized to something that … |
|
|
The /> must be a >. |
|
|
for must align with class. |
|
|
This must also be <%-. |
|
|
This is indented 1 space too few. |
|
|
Rather than defining the variables above, you can pass in the strings here directly. |
|
|
Missing comma after this last line in the list. |
|
|
This could be one statement. |
|
|
This could be one statement. |
|
|
Doc summaries must be a single line. You can go into more detail in the description. I'd just say "changed … |
|
|
Col: 31 Missing semicolon. |
![]() |
Commits: |
|
||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 2 (+292 -42) |
Description: |
|
---|
Description: |
|
---|
Testing Done: |
|
|||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Commits: |
|
|||||||||||||||||||||||||
Diff: |
Revision 3 (+348 -58) |
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 3) |
---|
Col: 97 Missing semicolon.
Description: |
|
|||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Commits: |
|
|||||||||||||||||||||||||||||||
Branch: |
|
|||||||||||||||||||||||||||||||
Diff: |
Revision 4 (+349 -59) |
I think, personally, this UI fits in really nicely with Review Board's pre-existing metaphors. Great job!
A few notes:
- We probably still want the filename in the center of this, rather than pushed to the right.
- The labels should be vertical-center aligned
- Either both should end with
:
or not.- We probably want a little extra space between "Custom" and the image on its left - maybe the same amount of distance between the background color selectors.
Commits: |
|
|||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 5 (+362 -62) |
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 5) |
---|
Col: 45 Missing semicolon.
Commits: |
|
|||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 6 (+363 -63) |
Commits: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 7 (+366 -64) |
Commits: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 8 (+443 -165) |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||
Removed Files: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Added Files: |
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 8) |
---|
Col: 67 Missing semicolon.
Description: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Commits: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 9 (+444 -166) |
Testing Done: |
|
---|
You're working with some older code that predates our modern CSS naming conventions, but it'd still be good to make use of them for all new class names. See our documentation on that.
reviewboard/static/rb/css/pages/image-review-ui.less (Diff revision 9) |
---|
The
text-align
should predecede the nestedtable
rule, because it's relevant to.review-ui-header
.You'd also want a blank line between that and nested rules.
reviewboard/static/rb/css/pages/image-review-ui.less (Diff revision 9) |
---|
Can you put this in alphabetical order? While not always appropriate for all CSS rules, it's a good point to start. The exception to it is that we group related properties (
width
/height
,position
/top
/right
/bottom
/left
, etc.) together.This applies to other rules below.
reviewboard/static/rb/css/pages/image-review-ui.less (Diff revision 9) |
---|
This is too generic a name, so it'll be especially important to namespace this, as per our docs.
reviewboard/static/rb/css/pages/image-review-ui.less (Diff revision 9) |
---|
Can you do testing on Firefox and Safari for these?
reviewboard/static/rb/js/models/imageReviewableModel.es6.js (Diff revision 9) |
---|
Can you reorder these to be in alphabetical order?
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 9) |
---|
The blank line here should stay.
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 9) |
---|
Some important comments here:
- Keep line lengths in mind. Lines cannot go beyond 79 characters.
- These sorts of CSS changes are best done in CSS, not in JavaScript.
- Always use
const
orlet
instead ofvar
.
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 9) |
---|
Blank line between these.
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 9) |
---|
Space after the
if
, before the(
.
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 9) |
---|
Can you go through the file and make sure there's no extra whitespace (identified by these red blocks)?
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 9) |
---|
This would be best done as a
_.template
somewhere, using_.dedent
(see other parts of the codebase for examples).
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 9) |
---|
Any text must be localized and provided in. This is easier done with templates. You can localize a string using:
_`Background`
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 9) |
---|
Always use double quotes for attribute values, and never self-close tags.
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 9) |
---|
You can use chaining to reduce the code here:
$('<div class="....">') .append($backgroundMenu) .append($resolutionMenu) .appendTo($header);
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 9) |
---|
You can set multiple model attributes at once:
this.model.set({ checkeredBackground: false, background: color, });
This helps with ensuring that signal handlers can get the new set of attributes at the same time, rather than triggering a bunch of different signals and leaving it up to the code to sort it out.
While not an issue necessarily right here, it's a pattern that's good to adopt for Backbone.
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 9) |
---|
This can be one statement.
Also, code should always use single quotes instead of double quotes, unless the text in the string requires a single quote.
These apply to code below.
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 9) |
---|
Missing a period.
This must also be one line (and can't go beyond the line length).
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 9) |
---|
This should probably be driven by the model.
The model should listen for a change to
checkeredBackground
. If set, it should set the background to a value.It should probably also go the other way. If explicitly setting a background, unset
checkeredBackground
. That would simplify code above.
Commits: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 10 (+542 -264) |
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 10) |
---|
Col: 12 Unexpected ')'.
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 10) |
---|
Col: 12 Expected an identifier and instead saw ')'.
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 10) |
---|
Col: 13 Expected ')' and instead saw ';'.
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 10) |
---|
Col: 14 Missing semicolon.
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 10) |
---|
Col: 15 '$toolBar' is defined but never used.
Commits: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 11 (+567 -285) |
Commits: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 12 (+571 -289) |
Commits: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 13 (+587 -301) |
Summary: |
|
||||||
---|---|---|---|---|---|---|---|
Testing Done: |
|
reviewboard/static/rb/css/pages/image-review-ui.less (Diff revision 13) |
---|
Let's use
#rb-ns-ui.colors[@grey-40]
instead of#808080
reviewboard/static/rb/css/pages/image-review-ui.less (Diff revision 13) |
---|
This should also use one of the
#rb-ns-ui.colors
definitions (seereviewboard/static/rb/css/ui/colors.less
) instead of "grey"
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 13) |
---|
Please use single quotes instead of double.
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 13) |
---|
Please use single quotes instead of double.
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 13) |
---|
This should probably be called
backgroundMenuTemplate
. The$
prefix is for jQuery objects.
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 13) |
---|
We could skip defining this as a variable and just do:
$('<div class="...">') .append(...)
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 13) |
---|
Let's wrap this like so:
.append(backgroundMenuTemplate({ backgroundLabel: backgroundLabel, customLabel: customLabel, }) .append($resolutionMenu) ...
Note that we also don't need quotes around the keys that we're passing in there.
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 13) |
---|
Please add a trailing comma at the end of this.
Description: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Testing Done: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Commits: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 14 (+2200 -4598) |
Commits: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 15 (+614 -320) |
Hi Matthew! It is an awesome feature that you implement!
I was wondering why in the styling sheets, sometimes you are using the tag identifier started with the & sign, such as
&.-black
and&.-white
, instead of the typical.-white
? Also, for the checkered background selection, why do we need to specify the background-position and background-size, while the other selections don't?
I think we should have the captions be last.
We want to think of the UI in logical components. We have here, in order:
- Controls for manipulating the view
- Labels that describe the content below
- Controls for manipulating the view
- Content
It'd be much nicer as:
- Controls for manipulating the view
- Labels that describe the content below
- Content
reviewboard/static/rb/css/pages/image-review-ui.less (Diff revision 15) |
---|
I only see one place each where we use these. We don't need to define new variables for them. Let's put them where they're needed.
reviewboard/static/rb/css/pages/image-review-ui.less (Diff revision 15) |
---|
A more standardized name (in our modern standard) would be
-has-checkered-background
.
reviewboard/static/rb/css/pages/image-review-ui.less (Diff revision 15) |
---|
We usually make
display
the first, or one of the first, properties in a rule. Can you move it up?
reviewboard/static/rb/css/pages/image-review-ui.less (Diff revision 15) |
---|
Where possible, let's use
em
units. These are going to scale better than pixel units, making them more accessible for users.Usually we have units in increments of
0.5em
, depending on what we're displaying (e.g.,1em
,2em
,1.5em
, etc.). Not a hard-and-fast rule, but that does help keep a visual consistency.Same throughout the change.
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 15) |
---|
These are looking pretty long. Not uncommon for this to happen.
Try:
events: { ... 'click .rb-o-image-background-blah-blah-blah': '_onBlahChanged', ... }
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 15) |
---|
When you're setting a single attribute, pass each in as a parameter:
this.model.set('checkeredBackground', false);
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 15) |
---|
this.$el
is the.image-review-ui
component, so rather than scanning the entire DOM for it each time, you can just usethis.$el
.
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 15) |
---|
This can now be:
this.$el.toggleClass('-has-checkered-background', isCheckeredBackground);
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 15) |
---|
All this should be 1-space indentation.
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 15) |
---|
Only one space indentation.
This should also be
<%- backgroundLabel %>
, in case the text gets localized to something that isn't HTML-safe (<%-
will escape text).
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 15) |
---|
for
must align withclass
.
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 15) |
---|
This is indented 1 space too few.
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 15) |
---|
Rather than defining the variables above, you can pass in the strings here directly.
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 15) |
---|
Missing comma after this last line in the list.
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 15) |
---|
This could be one statement.
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 15) |
---|
This could be one statement.
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 15) |
---|
Doc summaries must be a single line.
You can go into more detail in the description.
I'd just say "changed to a checkered pattern."
Commits: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 16 (+647 -369) |
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 16) |
---|
Col: 31 Missing semicolon.
Commits: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 17 (+652 -370) |