Summary

Auto scale large images according to size

Review Request #8392 — Created Sept. 8, 2016 and submitted Sept. 19, 2016, 7:40 p.m.

Information

Owner

brennie

Repository

Review Board

Branch

release-3.0.x

Bugs

Depends On

~~8377~~

Reviewers

Groups

reviewboard

People

Description

Previously, we scaled images that contained @2x or @3x in their
filenames. For images that do not contain those hints, we now scale them
to the highest scaling factor less than or equal to 1 that allows the
most of the image to be shown.

Testing Done

Tested with a very large image and saw that it was scaled correctly in
image diff and single image mode.

Issues

Description	From	Last Updated
"Subclasses"	chipx86	Sept. 13, 2016, 9:53 a.m.
I know this was already here, but .includes isn't available in all browsers, and Babel doesn't do anything for it. …	chipx86	Sept. 12, 2016, 11:58 p.m.
Array.from isn't available in all browsers.	chipx86	Sept. 12, 2016, 11:58 p.m.
Can you put the comparison in parens? It'll help with making it clearer, given the => syntax. (Otherwise it almost …	chipx86	Sept. 13, 2016, 9:54 a.m.
These could be moved inline into the return statement with no loss of clarity.	david	Sept. 19, 2016, 7:17 p.m.
Same here.	david	Sept. 19, 2016, 7:17 p.m.

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



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

reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 1)
The issue has been resolved. Show all issues
```
"Subclasses"
```

reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 1)

The issue has been dropped. Show all issues

I know this was already here, but .includes isn't available in all browsers, and Babel doesn't do anything for it. We should use .indexOf instead.

david Sept. 12, 2016, 11:56 p.m.

We do use the babel polyfill, which adds this.

chipx86 Sept. 12, 2016, 11:58 p.m.

Ah, my apologies. I was running the wrong version of RB when checking that.

reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 1)
The issue has been dropped. Show all issues
```
Array.from isn't available in all browsers.
```
1. david Sept. 12, 2016, 11:56 p.m.
  Same here.

reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 1)

The issue has been resolved. Show all issues

Can you put the comparison in parens? It'll help with making it clearer, given the => syntax. (Otherwise it almost looks like Python's ability to do things like 3 >= 2 <= 5.)

Change Summary:

Addressed Christian's issues

Depends On:

+	~~8377 - Scroll oversized images in the image review UI~~

Diff:

Revision 2 (+154 -27)

Show changes

reviewboard/static/rb/js/views/imageReviewableView.es6.js

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



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

Ship it!

```
Ship It!
```

Change Summary:

Rebase

Diff:

Revision 3 (+155 -27)

Show changes

reviewboard/static/rb/js/views/imageReviewableView.es6.js

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



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

reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 3)
The issue has been resolved. Show all issues
```
These could be moved inline into the return statement with no loss of clarity.
```
reviewboard/static/rb/js/views/imageReviewableView.es6.js (Diff revision 3)
The issue has been resolved. Show all issues
```
Same here.
```

Change Summary:

Addressed David's issues

Diff:

Revision 4 (+149 -27)

Show changes

reviewboard/static/rb/js/views/imageReviewableView.es6.js

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



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

Status: Closed (submitted)

Change Summary:

Pushed to release-3.0.x (7ec56fb)