Skip to content
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

UX improvements #32

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

UX improvements #32

wants to merge 1 commit into from

Conversation

laukstein
Copy link

Shorter license, fix z-index, get large image src from anchor href, automatically zoom on mouseover.
Do not forget to update HTML and add <a href=large-image.jpg ....

Shorter license, fix `z-index`, get large image `src` from anchor `href`, automatically zoom on mouseover.
@jackmoore
Copy link
Owner

Thanks. I appreciate the pull request, and I am considering merging the shorter license and the assumption that the plugin is being assigned to anchor elements whose href is the zoomed image's src. Do you have a reason as to why mouseover would be better than mouseenter? I think mouseenter/mouseleave would be best because they don't trigger additional mouseenter/mouseleave events when mousing over child elements. I don't see why the plugin needs to assign a z-index. If someone wanted to change the layering via z-index, then that can be done at the stylesheet using the class name, instead of through the plugin. I imagine that the majority of the time users will want the z-index to auto because if a z-index was needed, they probably set it to the desired value for the target element.

@laukstein
Copy link
Author

While using mouseenter and not mouseover, it will not fire zoom when user has already on hover state, for example try to implement image zoom for full screen size element or to hover some element and refresh the page so that it will return your pointer on hover/mouseover state. The current event mouseenter requires to leave zoom element and after to mouseover it. By the way need to implement for it also touchstart and touchend event.

In case if img will have position: relative; without applying z-index for zoomed element, the zoom will not be visible.

@tylik1
Copy link

tylik1 commented Oct 6, 2013

I consider anchor tags with path to big image is bad for Seo. The link will never be visited and it should not be used this way. It is much better to use html5 data- attribute to store preferences about images.

@laukstein
Copy link
Author

@tylik1, you are wrong on considering SEO recommendations. It is fine to use image URL in links. With disabled JavaScript it would be accessible in standard way. Most of search engines does not understand JavaScript. It would be preferred to serve for search engines the largest resolution images than small thumbnails for better UX, after search engines can decide if to scale the images. HTML5 data- attribute is not compatible with SEO. Then better Microdata than data-.

@tylik1
Copy link

tylik1 commented Oct 7, 2013

@laukstein, What age are you living in? Why would one need to disable javascript today? If you need search-engines to find your image, than it's up to you, but I consider it to be wrong to have 2 same images with different size accessbile from one page.
Let's say you are running e-commerce store and show one scaled medium image of a product. Why would you need search engines to be able to find the same image with larger size?
No one will ever need to use this link. All links on the page have some seo weight and it's a waste from that point.
I want to use html5 data- attribute to hide the big image from search engines, instead of using href attribute.

@laukstein
Copy link
Author

@tylik1, do you really understand how searching engines work. I agree with you that the best is to serve to crawlers just one unique image (the same with textual articles) and no duplication even if the difference is in size. To restrict searching engines use robots.txt like

User-agent: *
Disallow: /thumb/
Disallow: /images/*.thumb.jpg$

Google tries hard to support simple JavaScript's in Googleboot, and it is hard to say if any other search engine boot has any support for JavaScript.

If you meant to use HTML/CSS reduced image and zoom the same image up to real size, than it is totally against to SEO and UX recommendations.

@tylik1
Copy link

tylik1 commented Oct 7, 2013

If you meant to use HTML/CSS reduced image and zoom the same image up to real size, than it is totally against to SEO and UX recommendations.

No I didn't

I understand that you can use robots.txt, but I don't really see point in it. I already have a medium sized image on a page, and I think it is enough, and as you said there shouldn't be dublicates. A link with larged-sized same image is an overkill for me. Even if search engine uderstands javascript, there is a big difference in putting link with image on a page, and using data- attribute to get the image.
I could put rel="nofollow" to the link with big image, and that would solve my problem, but I don't really think I should.
Much easier for me is not to create any link at all.
Actually I think we would both be happy if one could choose attribute in options, which will determine path to an image. Something like:
urlAttribute : 'data-url'
Or as you prefer:
urlAttribute : 'href'

@laukstein
Copy link
Author

I agree with you, urlAttribute: 'data-url' would be great or even better use custom selector like zoomLarge: '.zoomimg [data-img]'.

@tylik1
Copy link

tylik1 commented Oct 7, 2013

ZoomLarge: '.zoomimg[data-img]'

Nice! This is the way to go!
The only thing is that there should be no space between selector and attribute. Otherwise Jquey will not find it.
$('.zoomimg[data-img]') but not $('.zoomimg [data-img]')

@laukstein
Copy link
Author

If you are right, then it is jQuery selector bug.

@Mithgol
Copy link

Mithgol commented Oct 8, 2013

There's no bug; .zoomimg [data-img] means some [data-img] element inside a .zoomimg element — just like A B is some B inside a hypelink. Whitespace matters in CSS.

@laukstein
Copy link
Author

I agree with @Mithgol, but @tylik1 says it does not work for jQuery. .zoomimg [data-img] is the same as .zoomimg *[data-img] if it would solve the jQuery bug.

@tylik1
Copy link

tylik1 commented Oct 8, 2013

<<I agree with @Mithgol, but @tylik1 says it does not work for jQuery. .zoomimg [data-img] is the same as .zoomimg *[data-img] if it would solve the jQuery bug.

Sorry I thought you wanted to select the element directly containing certain attribute, but if you want to select elements descendants with specified attributes, then it will work fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants