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>