Summary

Defining a mode for imageAttachmentView

Review Request #7772 — Created Nov. 15, 2015 and submitted Nov. 16, 2015, 12:10 p.m.

Information

Owner

chronicleyu

Repository

Review Board

Branch

master

Bugs

Depends On

Reviewers

Groups

reviewboard, students

People

brennie, chipx86, david, mike_conley

Description

When rendering the image review view, the div class for the image itself in imageAttachmentView mode currently has name image-diff-null. This is because the base class BaseImageView initializes mode to null but the subclass imageAttachmentView does not define a proper name for it. This can potentially confuse devs, or at the very least, it gives the impression that the class was not extended properly.
This patch fixes the problem by setting the mode to attachment so that the div class will be image-diff-attachment rather than image-diff-null.

Testing Done

Used Inspect Element tool on the browser to ensure that the div class now reads image-diff-attachment.

Tool: Pyflakes
Ignored Files:
    reviewboard/static/rb/js/views/imageReviewableView.js



Tool: PEP8 Style Checker
Ignored Files:
    reviewboard/static/rb/js/views/imageReviewableView.js

Change Summary:: Fixed an error in the description.
Description:: ~
Defined a mode for imageAttachmentView so that div class is not image-diff-attachment.
~
Defined a mode for imageAttachmentView so that div class is not image-diff-null.

This is a pretty simple patch, but it needs a better change description.

Please include why this is an issue. You may want to read this guide on writing descriptions.

Change Summary:: Refined the description to give a more detailed explanation of the null issue in the div class naming and how it was fixed.
Summary:: Bug fix: Defined a mode for imageAttachmentView
Defining a mode for imageAttachmentView
Description:: ~
Defined a mode for imageAttachmentView so that div class is not image-diff-null.
~
When rendering the image review view, the div class for the image itself in imageAttachmentView mode currently has name image-diff-null. This is because the base class BaseImageView initializes mode to null but the subclass imageAttachmentView does not define a proper name for it. This can potentially confuse devs, or at the very least, it gives the impression that the class was not extended properly.
+
+
This patch fixes the problem by setting the mode to attachment so that the div class will be image-diff-attachment rather than image-diff-null.
Testing Done:: ~
Inspect Element on browser. Div class now reads image-diff-attachment.
~
Used Inspect Element tool on the browser to ensure that the div class now reads image-diff-attachment.

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to master (3c287d8)

~		Defined a mode for imageAttachmentView so that div class is not `image-diff-attachment`.
	~	Defined a mode for imageAttachmentView so that div class is not `image-diff-null`.

~		Defined a mode for imageAttachmentView so that div class is not `image-diff-null`.
	~	When rendering the image review view, the div class for the image itself in imageAttachmentView mode currently has name `image-diff-null`. This is because the base class `BaseImageView` initializes `mode` to `null` but the subclass `imageAttachmentView` does not define a proper name for it. This can potentially confuse devs, or at the very least, it gives the impression that the class was not extended properly.
	+
	+	This patch fixes the problem by setting the `mode` to `attachment` so that the div class will be `image-diff-attachment` rather than `image-diff-null`.

~		Inspect Element on browser. Div class now reads `image-diff-attachment`.
	~	Used Inspect Element tool on the browser to ensure that the div class now reads `image-diff-attachment`.