Skip to content

Commit 7b21920

Browse files
cslzchenfelliott
authored andcommitted
Several major and minor updates:
- Updated the main script in viewer.mako - Use a better and future-compatible mobile detection solution based on window size - Images displayed with actual size no longer needs a second layer of wrapping. It now shares the same default way of wrapping as downsized but not-in-chrome images - `.zoom()` is called with the magification scale and the mouse action "click" - Several minor coding and style improvements - Updated jquery.zoom.js customizations - Removed customization for `defaults`, mouse action is now set when the main script above calls `.zoom()` - Remove `{overflow: "hidden"}` customization for the zoom image. It simply has no effect at all - Move `stop()` out of the `if (cliked) {}` statement and update the comments with TODOs - Tested every "otherwise" failure case so that I was able to add detailed comments in viewer.mako on WHAT, HOW and WHY it works the way I implemented it - Updated tests
1 parent 1b13ec3 commit 7b21920

File tree

4 files changed

+116
-110
lines changed

4 files changed

+116
-110
lines changed

mfr/extensions/image/static/js/jquery.detectmobile.js

Lines changed: 0 additions & 7 deletions
This file was deleted.

mfr/extensions/image/static/js/jquery.zoom.js

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,11 @@
33
*
44
* Original Copy: https://github.com/jackmoore/zoom/blob/1.7.20/jquery.zoom.js
55
* Version: https://github.com/jackmoore/zoom/releases/tag/1.7.20
6-
* Changes: customized for MFR and updated style
6+
*
7+
* The are two MFR customizations in this file, one for style and one for functionality
8+
*
9+
* 1. Updated code style according to `.eslintrc.js`
10+
* 2. Added `.on("mousewheel", function (e) { ... })` to enable further zoom by mouse wheel scroll
711
*/
812

913
(function ($) {
@@ -15,8 +19,8 @@
1519
callback: false,
1620
target: false,
1721
duration: 120,
18-
on: "click", // Decides when to enlarge. Options: grab, click, toggle and mouseover
19-
touch: true, // Enables a touch fallback.
22+
on: "mouseover",
23+
touch: true,
2024
onZoomIn: false,
2125
onZoomOut: false,
2226
magnify: 1
@@ -53,8 +57,7 @@
5357
height: img.height * magnify,
5458
border: "none",
5559
maxWidth: "none",
56-
maxHeight: "none",
57-
overflow: "hidden" // TODO: How does this customization affect the rendering?
60+
maxHeight: "none"
5861
}).appendTo(target);
5962

6063
return {
@@ -162,7 +165,6 @@
162165

163166
} else if (settings.on === "click") {
164167

165-
// Added "mousewheel" event to enable further zoom when the images are enlarged
166168
$source.on("click.zoom", function (e) {
167169
if (!clicked) {
168170
clicked = true;
@@ -179,9 +181,15 @@
179181
}
180182
// do nothing if clicked is true, bubble the event up to the document to
181183
// trigger the unbind.
182-
}).on("mousewheel", function (e) {
184+
});
185+
186+
// MFR customization: Allow further zoom using mouse wheel on the zoom image.
187+
// The zoom is only enabled when image is clicked. I am not entirely sure how
188+
// `stop()` works but having it inside or outside the `if (clicked) { ... }`
189+
// statement does not make a difference. TODO: need more investigation
190+
$source.on("mousewheel", function (e) {
191+
stop();
183192
if (clicked) {
184-
stop();
185193
start(e);
186194
}
187195
});

mfr/extensions/image/templates/viewer.mako

Lines changed: 100 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,8 @@
33
<link rel="stylesheet" type="text/css" href="/static/css/bootstrap.min.css"/>
44
<script src="/static/js/bootstrap.min.js"></script>
55

6-
## Quirks 0:
7-
##
8-
## Need to add this block buffer so that the question circle does not overlap with the image. The
9-
## height will be set if zoom feature is enabled for the give image. The height is set to be 38px,
10-
## which includes 10px each for top and bottom margin, and 18px for the question circle icon.
11-
##
6+
## The block to hold the question circle, which prevents it from overlapping with the image and
7+
## of which the style will be updated if zoom is enabled
128
<div id="popover-buffer" style="display: none"></div>
139
<a id="popover-content" style="display: none" data-toggle="popover"
1410
data-trigger="hover" data-placement="left" data-html="true"
@@ -17,115 +13,109 @@
1713
<p><b> Zoom:</b> Click on the image and use the mouse wheel to zoom in and out</p>
1814
<p><b> Navigate:</b> Move the mouse cursor to navigate through the magnified image</p>
1915
"
20-
>
21-
<img src="${base}/images/question-circle.png">
22-
</a>
16+
><img src="${base}/images/question-circle.png"></a>
2317

18+
## The image to render, which will be wrapped accordingly if zoom is enabled
2419
<img id="base-image" style="max-width: 100%" class="baseImage" src="${url}">
2520

21+
## MFR scripts
2622
<script src="/static/js/mfr.js"></script>
2723
<script src="/static/js/mfr.child.js"></script>
2824

29-
<script src="${base}/js/jquery.detectmobile.js"></script>
25+
## JQuery Zoom and Mouse Wheel scripts
3026
<script src="${base}/js/jquery.mousewheel.min.js"></script>
3127
<script src="${base}/js/jquery.zoom.js"></script>
3228

29+
## The main script for enable Hi-Res and Zoom for rendered images
3330
<script>
34-
31+
## Enforces strict mode
3532
"use strict";
3633
34+
## Magnifiation parameters
3735
var magScale = 1;
3836
var magStep = 0.1;
3937
var maxScale = 3;
4038
var minScale = 1;
41-
4239
var magHeight = null;
4340
var magWidth = null;
4441
42+
## The height and width for the displaying part of the image
4543
var heightLimit = null;
4644
var widthLimit = null;
4745
48-
var heightThreshold = 200;
46+
## The minimal height requirement to enable the zoom feature for a image
47+
## For more information, refer to the line where it is used in this file
48+
var minHeightToZoom = 200;
4949
50+
## A flag indicating whether the image is already wrapped or not
5051
var wrapped = false;
5152
52-
## Enable the zoom feature only for desktop browsers
53-
if (!$.browser.mobile) {
53+
$(document).ready(function() {
5454
55-
var popoverBuffer = $("#popover-buffer");
56-
var popoverContent = $("#popover-content");
55+
## Reference on how mobile is detected: https://stackoverflow.com/a/10364620
56+
var isMobile = window.matchMedia("only screen and (max-width: 760px)");
5757
58-
popoverBuffer.css({
59-
"height": "38px"
60-
});
58+
## Enable the zoom feature only for desktop browsers
59+
if (!isMobile.matches) {
6160
62-
popoverContent.css({
63-
"position": "absolute",
64-
"top": "10px",
65-
"right": "10px",
66-
"cursor": "pointer"
67-
});
61+
## Update the style for instruction popover and enable it
62+
var popoverBuffer = $("#popover-buffer");
63+
var popoverContent = $("#popover-content");
64+
popoverBuffer.css({"height": "38px"});
65+
popoverContent.css({
66+
"position": "absolute",
67+
"top": "10px",
68+
"right": "10px",
69+
"cursor": "pointer"
70+
});
71+
$('[data-toggle="popover"]').popover();
72+
73+
var baseImage = $("#base-image");
6874
69-
$('[data-toggle="popover"]').popover();
75+
## Quirks: the base image must be wrapped, see http://www.jacklmoore.com/zoom/
76+
##
77+
## The BASE image must be wrapped with a parent "container" and it must be the only (or
78+
## the first) image in it. The `jquery.zoom` library performs the zoom on the parent
79+
## and grabs the first image it finds to create the zoom image.
80+
##
81+
## `addSpan()` performs the "wrap" accordingly to cater for most cases
82+
var addSpan = function () {
83+
84+
## Only wrap the image once
85+
if (wrapped) {
86+
return;
87+
}
7088
71-
$(document).ready(function() {
89+
## Obtain the original height and width for the image loaded
90+
var baseImageHeight = parseInt(baseImage.css("height"), 10);
91+
var baseImageWidth = parseInt(baseImage.css("width"), 10);
7292
73-
var is_chrome = !!window.chrome;
93+
## Detect the Google Chrome browser
94+
var isChrome = !!window.chrome;
7495
75-
var baseImage = $("#base-image");
96+
## Detect if the image is downsized due to screen size
97+
var isActualSize = heightLimit === baseImageHeight || widthLimit === baseImageWidth;
7698
77-
var addSpan = function() {
78-
79-
if (!wrapped) {
80-
81-
var baseImageHeight = parseInt(baseImage.css("height"));
82-
var baseImageWidth = parseInt(baseImage.css("width"));
83-
84-
## Quirk 1
85-
##
86-
## The BASE image must be wrapped with a parent "container" and it must be the
87-
## only image in this parent. The "JQuery Zoom" library performs the zoom
88-
## function on the parent, in which it uses the first image it finds to create
89-
## the ZOOM image.
90-
##
91-
## There are two issues if it is not wrapped. First, the ZOOM image will be the
92-
## question cirle, which is the first image we have in the iframe. Second, the
93-
## ZOOM image will take over the full space of the iframe.
94-
##
95-
## Quirk 2
96-
##
97-
## Wrapping <img> with <div> or <span> in Google Chrome makes the scroll bar in
98-
## the <iframe> flickering when zoom is active. However, wrap it with <p> does
99-
## not have the problem for most of the time.
100-
##
101-
## Quirk 3
102-
##
103-
## Weird behaviors SOMETIMES occur when the image is downsized due to smaller
104-
## screen size. Thus, different wrapping are used.
105-
##
106-
## Quirk 4
107-
##
108-
## Werid behaviors ALWAYS occur when the image's height is less than 150. This
109-
## number is obtained by experiments. Thus, zoom are disabled for such images.
110-
##
111-
if (heightLimit === baseImageHeight || widthLimit === baseImageWidth) {
112-
baseImage.wrap("<div></div>").parent().css({
113-
"display": "block",
114-
"position": "relative",
115-
"overflow": "hidden"
116-
});
117-
baseImage.parent().wrap("<span></span>").parent().css("display", "inline-block");
118-
} else if (!is_chrome) {
119-
baseImage.wrap("<div></div>").parent().css("display", "inline-block");
120-
} else {
121-
baseImage.wrap("<p></p>")
122-
}
123-
124-
wrapped = true;
99+
if (isActualSize || !isChrome) {
100+
## Use the default wrapping suggested by http://www.jacklmoore.com/zoom/ if
101+
## either of the two conditions below holds:
102+
## 1. Images are downsized but not in Google Chrome. Please see the flickering
103+
## issue mentioned below.
104+
## 2. Images are displayed in its actual size. No issue for all supported
105+
## browsers.
106+
baseImage.wrap("<div></div>").parent().css("display", "inline-block");
107+
} else {
108+
## Quirks: Chrome has a flickering bug when images are wrapped with `<div>` or
109+
## `<span>`. During zoom the scroll bar keeps appearing and disappearing which
110+
## causes the image to keep resizing. Wrapping with `<p>` instead to solve this
111+
## annoying issue.
112+
baseImage.wrap("<p></p>")
125113
}
114+
115+
wrapped = true;
126116
};
127117
128-
var mouseZoom = function(event, delta) {
118+
var mouseZoom = function (event, delta) {
129119
130120
## Disable page scroll when the mouse cursor are above the image
131121
event.preventDefault();
@@ -139,30 +129,48 @@
139129
}
140130
141131
var image = $(".zoomImage");
142-
magHeight = magHeight === null ? parseInt(image.css("height")) : magHeight;
143-
magWidth = magWidth === null ? parseInt(image.css("width")) : magWidth;
132+
magHeight = magHeight === null ? parseInt(image.css("height"), 10) : magHeight;
133+
magWidth = magWidth === null ? parseInt(image.css("width"), 10) : magWidth;
144134
145135
image.css({
146136
width: magWidth * magScale,
147137
height: magHeight * magScale
148138
});
149139
};
150140
151-
## Create an in memory copy of the image to avoid CSS issues, TODO: double check
152-
$("<img>").attr("src", $(baseImage[0]).attr("src")).load(function() {
141+
## Quirks: Need to use an in-memory copy of the image to prevent the zoom image from
142+
## moving around. Without this in-memoery copy, the zoom image moves around
143+
## horizontally and "centers" wherever the mouse cursor is.
144+
$("<img>").attr("src", $(baseImage[0]).attr("src")).load(function () {
153145
154146
widthLimit = this.width;
155147
heightLimit = this.height;
156148
157-
## Enable zoom and display popover instructions only when images are eligible
158-
if (heightLimit >= heightThreshold) {
159-
$(window).resize(addSpan);
160-
$("#popover-buffer").css("display", "block");
161-
$("#popover-content").css("display", "block");
162-
addSpan();
163-
baseImage.parent().zoom({magnify: magScale}).on("mousewheel", mouseZoom);
149+
## Quirks: Disable zoom and display instructions for images of height less than 200.
150+
## 1. The original issue was when the height is less than 150 (not 200), the zoom
151+
## image moves around and thus does not cover the base image fully. However,
152+
## I cannot trigger this any more.
153+
## 2. When the height is less than around 200 (a little bit less than 200), which
154+
## is close to the height of the popover instruction when shown, the bottom
155+
## part of the instruction block is cut off. Users need to scroll down to see
156+
## the full message. In addition, the scroll bar overlaps with the question
157+
## circle.
158+
## 3. Images of height less than 200 are not worth zooming anyway.
159+
if (heightLimit < minHeightToZoom) {
160+
return;
164161
}
162+
163+
## For images that are eligible to zoom:
164+
## 1. Display the question circle for popover zoom instructions
165+
## 2. Wrap the base image accordingly by calling `addSpan()`
166+
## 3. Call zoom on its parent with customized configs (scale and mouse click)
167+
## 4. Enable further zoom on mouse wheel
168+
$(window).resize(addSpan);
169+
$("#popover-buffer").css("display", "block");
170+
$("#popover-content").css("display", "block");
171+
addSpan();
172+
baseImage.parent().zoom({magnify: magScale, on: 'click'}).on("mousewheel", mouseZoom);
165173
});
166-
});
167-
}
174+
}
175+
});
168176
</script>

tests/extensions/image/test_renderer.py

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import pytest
2-
import logging
32

43
import furl
54

@@ -8,8 +7,6 @@
87
from mfr.extensions.image import ImageRenderer
98
from mfr.extensions.image import settings
109

11-
logger = logging.getLogger(__name__)
12-
1310

1411
class TestImageRenderer:
1512

0 commit comments

Comments
 (0)