Fix inconsistent height and width in opacity mode
The _height and _width fields are not reliably set by Gerrit. Rather
than relying on them for dimension computation, compute them manually.
Change-Id: I6b987a0846c175814f9386ce91f9075b64473f67
diff --git a/gr-opacity-diff-mode/gr-opacity-diff-mode.html b/gr-opacity-diff-mode/gr-opacity-diff-mode.html
index c4604e4..ea16a86 100644
--- a/gr-opacity-diff-mode/gr-opacity-diff-mode.html
+++ b/gr-opacity-diff-mode/gr-opacity-diff-mode.html
@@ -32,7 +32,7 @@
}
#imageRevision {
opacity: var(--my-opacity-value);
- z-index: 1;
+ z-index: 0.5;
}
#imageDiffContainer {
height: var(--div-height);
@@ -51,14 +51,12 @@
width: 100%;
}
label {
+ align-items: center;
+ display: flex;
padding: 1em .5em;
}
input {
- padding: 1em 0;
- }
- .cell {
- align-items: center;
- display: flex;
+ margin: .5em;
}
#opacitySlider {
width: 10em;
@@ -66,24 +64,20 @@
</style>
<div class="wrapper">
<div id="imageDiffContainer">
- <img id="imageBase"/>
- <img data-opacity$="{{opacityValue}}" id="imageRevision"/>
+ <img on-load="_onImageLoad" id="imageBase"/>
+ <img on-load="_onImageLoad" data-opacity$="{{opacityValue}}" id="imageRevision"/>
</div>
<div id="controlsContainer">
<div id="controlsBox">
- <div class="cell">
+ <label>
<input
id="scaleSizesToggle"
on-tap="handleScaleSizesToggle"
type="checkbox">
- <label for="scaleSizesToggle">
- Scale to same size
- </label>
- </div>
- <div class="cell">
- <label for="opacitySlider">
- Revision image opacity
- </label>
+ Scale to same size
+ </label>
+ <label>
+ Revision image opacity
<input
id="opacitySlider"
max="1.0"
@@ -92,10 +86,10 @@
step=".01"
type="range"
value="0.5"/>
- </div>
+ </label>
</div>
</div>
</div>
</template>
- <script src="gr-opacity-diff-mode.js"></script>
+ <script src="gr-opacity-diff-mode.js"></script>
</dom-module>
diff --git a/gr-opacity-diff-mode/gr-opacity-diff-mode.js b/gr-opacity-diff-mode/gr-opacity-diff-mode.js
index 6855d0c..3574387 100644
--- a/gr-opacity-diff-mode/gr-opacity-diff-mode.js
+++ b/gr-opacity-diff-mode/gr-opacity-diff-mode.js
@@ -20,19 +20,35 @@
baseImage: Object,
revisionImage: Object,
opacityValue: Number,
+ _maxHeight: {
+ type: Number,
+ value: 0,
+ },
+ _maxWidth: {
+ type: Number,
+ value: 0,
+ },
},
- attached() {
- if (this.revisionImage) {
- const srcRevision = this.computeSrcString(this.revisionImage);
- this.$.imageRevision.setAttribute('src', srcRevision);
- this.handleOpacityChange();
- }
- if (this.baseImage) {
- const srcBase = this.computeSrcString(this.baseImage);
- this.$.imageBase.setAttribute('src', srcBase);
- }
- this.resizeDiffContainerHeight();
+ observers: [
+ '_handleImageChange(baseImage, revisionImage)',
+ '_handleHeightChange(_maxHeight)',
+ '_handleWidthChange(_maxWidth)',
+ ],
+
+ _onImageLoad(e) {
+ this._maxHeight = Math.max(this._maxHeight,
+ Polymer.dom(e).rootTarget.naturalHeight);
+ this._maxWidth = Math.max(this._maxWidth,
+ Polymer.dom(e).rootTarget.naturalWidth);
+ },
+
+ _handleImageChange(baseImage, revisionImage) {
+ this.$.imageRevision.setAttribute('src',
+ this.computeSrcString(this.revisionImage));
+ this.$.imageBase.setAttribute('src',
+ this.computeSrcString(this.baseImage));
+ this.handleOpacityChange();
},
handleOpacityChange() {
@@ -47,25 +63,25 @@
handleScaleSizesToggle() {
let width;
let height;
- if (this.$.scaleSizesToggle.checked &&
- this.baseImage && this.revisionImage) {
- width = Math.max(this.revisionImage._width, this.baseImage._width);
- height = Math.max(this.revisionImage._height, this.baseImage._height);
+ if (this.$.scaleSizesToggle.checked) {
+ width = this._maxWidth;
+ height = this._maxHeight;
}
+
this.customStyle['--img-width'] = width ? width + 'px' : null;
this.customStyle['--img-height'] = height ? height + 'px' : null;
this.updateStyles();
},
- resizeDiffContainerHeight() {
- const maxHeight = Math.max(
- this.baseImage ? this.baseImage._height : 0,
- this.revisionImage ? this.revisionImage._height : 0);
- this.customStyle['--div-height'] = maxHeight + 'px';
- const maxWidth = Math.max(
- this.baseImage ? this.baseImage._width : 0,
- this.revisionImage ? this.revisionImage._width : 0);
- this.customStyle['--div-width'] = maxWidth + 'px';
+ _handleHeightChange(height) {
+ if (!height) { return; }
+ this.customStyle['--div-height'] = height + 'px';
+ this.updateStyles();
+ },
+
+ _handleWidthChange(width) {
+ if (!width) { return; }
+ this.customStyle['--div-width'] = width + 'px';
this.updateStyles();
},
});
diff --git a/gr-opacity-diff-mode/gr-opacity-diff-mode_test.html b/gr-opacity-diff-mode/gr-opacity-diff-mode_test.html
index a12913f..b98c19b 100644
--- a/gr-opacity-diff-mode/gr-opacity-diff-mode_test.html
+++ b/gr-opacity-diff-mode/gr-opacity-diff-mode_test.html
@@ -37,9 +37,6 @@
let element;
setup(() => {
- stub('gr-opacity-diff-mode', {
- attached() {},
- });
element= fixture('opacity-diff-fixture');
});
@@ -63,72 +60,26 @@
expectedOutcome);
});
- suite('size scaling', () => {
- test('base image larger', () => {
- element.baseImage = {
- _width: 200,
- _height: 300,
- };
- element.revisionImage = {
- _width: 100,
- _height: 90,
- };
- assert.equal(element.customStyle['--img-width'], undefined);
- assert.equal(element.customStyle['--img-height'], undefined);
- element.$.scaleSizesToggle.checked = true;
- element.handleScaleSizesToggle();
- flushAsynchronousOperations();
- assert.equal(element.customStyle['--img-width'], '200px');
- assert.equal(element.customStyle['--img-height'], '300px');
- });
+ test('size scaling', () => {
+ element._maxWidth = 200;
+ element._maxHeight = 400;
- test('revisionImage larger', () => {
- element.baseImage = {
- _width: 200,
- _height: 300,
- };
- element.revisionImage = {
- _width: 400,
- _height: 500,
- };
- assert.equal(element.customStyle['--img-width'], null);
- assert.equal(element.customStyle['--img-height'], null);
- element.$.scaleSizesToggle.checked = true;
- element.handleScaleSizesToggle();
- flushAsynchronousOperations();
- assert.equal(element.customStyle['--img-width'], '400px');
- assert.equal(element.customStyle['--img-height'], '500px');
- });
-
- test('bigger base width, smaller base height', () => {
- element.baseImage = {
- _width: 200,
- _height: 300,
- };
- element.revisionImage = {
- _width: 140,
- _height: 400,
- };
- assert.equal(element.customStyle['--img-width'], null);
- assert.equal(element.customStyle['--img-height'], null);
- element.$.scaleSizesToggle.checked = true;
- element.handleScaleSizesToggle();
- flushAsynchronousOperations();
- assert.equal(element.customStyle['--img-width'], '200px');
- assert.equal(element.customStyle['--img-height'], '400px');
- });
- });
- test('resize the div container for base & revision image comparison', () => {
- element.baseImage = {
- _height: 500,
- };
- element.revisionImage = {
- _height: 300,
- };
- assert.equal(element.customStyle['--div-height'], undefined);
- element.resizeDiffContainerHeight();
+ assert.equal(element.customStyle['--img-width'], null);
+ assert.equal(element.customStyle['--img-height'], null);
+ MockInteractions.tap(element.$.scaleSizesToggle);
flushAsynchronousOperations();
+
+ assert.equal(element.customStyle['--img-width'], '200px');
+ assert.equal(element.customStyle['--img-height'], '400px');
+ });
+
+ test('resize the div container for base & revision image comparison', () => {
+ assert.equal(element.customStyle['--div-height'], undefined);
+ element._maxHeight = 500;
+ element._maxWidth = 300;
+ flushAsynchronousOperations();
+
assert.equal(element.customStyle['--div-height'], '500px');
});
});
-</script>
\ No newline at end of file
+</script>
