-
Notifications
You must be signed in to change notification settings - Fork 211
New plugin: theguardian.com #1782
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,41 @@ | ||||||||||||||||
| var hoverZoomPlugins = hoverZoomPlugins || []; | ||||||||||||||||
| hoverZoomPlugins.push({ | ||||||||||||||||
| name:'theguardian', | ||||||||||||||||
| version:'0.1', | ||||||||||||||||
| prepareImgLinks:function (callback) { | ||||||||||||||||
| var res = []; | ||||||||||||||||
| var old_width = /width=[0-9]*/; | ||||||||||||||||
| var new_width = 'width=2000'; | ||||||||||||||||
|
|
||||||||||||||||
| // This handles article thumbnails on the main page, and images | ||||||||||||||||
| // inside articles. | ||||||||||||||||
| $('a[href^="/"], a[href^="#img"], a[href*="theguardian.com"').one('mouseenter', function () { | ||||||||||||||||
| var a = $(this); | ||||||||||||||||
| var data = a.data(); | ||||||||||||||||
|
|
||||||||||||||||
| if (data.hoverZoomSrc) { | ||||||||||||||||
| return; | ||||||||||||||||
| } | ||||||||||||||||
|
|
||||||||||||||||
| var img = a.parent().find('img[src*="i.guim.co.uk/"]'); | ||||||||||||||||
| var url = img.get(0).src; | ||||||||||||||||
|
Comment on lines
+20
to
+21
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If no matching 'img' element is found, 'img.get(0)' will be 'undefined', and attempting to access '.src' will throw a 'TypeError', crashing the event handler. You should add a defensive check to ensure the image element exists before accessing its properties.
Suggested change
|
||||||||||||||||
|
|
||||||||||||||||
| data.hoverZoomSrc = [url.replace(old_width, new_width)]; | ||||||||||||||||
|
|
||||||||||||||||
| a.addClass('hoverZoomLink'); | ||||||||||||||||
| hoverZoom.displayPicFromElement(a); | ||||||||||||||||
| }); | ||||||||||||||||
|
|
||||||||||||||||
| // This handles the profile picture of the article's author. | ||||||||||||||||
| // The width parameter is necessary. It doesn't accept | ||||||||||||||||
| // ridiculously large numbers, like 2100 and larger, but 2000 | ||||||||||||||||
| // seems fine. | ||||||||||||||||
| hoverZoom.urlReplace(res, | ||||||||||||||||
| 'img[src*="i.guim.co.uk/"]', | ||||||||||||||||
| old_width, | ||||||||||||||||
| new_width, | ||||||||||||||||
| ); | ||||||||||||||||
|
Comment on lines
+10
to
+37
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To solve the issue you mentioned in the PR description (where hovering anywhere inside the large blue link rectangle triggers the zoom instead of just the thumbnail), you can target the 'img' elements directly rather than the wrapping 'a' elements. HoverZoom's 'urlReplace' utility already handles image elements perfectly and binds the hover event directly to them. Since the article thumbnails and the author profile pictures both use the 'i.guim.co.uk' domain, you can simplify the entire plugin by using 'urlReplace' for all of them. This removes the need for the manual 'mouseenter' event listener entirely and solves your hover area issue. // This handles all images hosted on the Guardian's CDN (i.guim.co.uk),
// including article thumbnails, images inside articles, and author profile pictures.
hoverZoom.urlReplace(res,
'img[src*="i.guim.co.uk/"]',
old_width,
new_width
); |
||||||||||||||||
|
|
||||||||||||||||
| callback($(res), this.name); | ||||||||||||||||
| } | ||||||||||||||||
| }); | ||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a syntax error in the jQuery selector: 'a[href*="theguardian.com"' is missing its closing bracket ']'. It should be 'a[href*="theguardian.com"]'.