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

Allow user zoom #1733

Closed
wants to merge 1 commit into from
Closed

Allow user zoom #1733

wants to merge 1 commit into from

Conversation

chicgeek
Copy link
Contributor

Purpose: Update meta viewport attributes to improve accessibility by allowing user zoom. If developers need to limit zoom, they should be able to do so at the theme level.

In order to comply with WAI WCAG 2.0 AA accessibility requirements you should never disable pinch zoom:

**Purpose:** Update meta viewport attributes to improve accessibility by allowing user zoom. If developers need to limit zoom, they should be able to do so at the theme level.

In order to comply with WAI WCAG 2.0 AA accessibility requirements you should never disable pinch zoom:

* W3C:
http://www.w3.org/TR/mobile-accessibility-mapping/#zoom-magnification
* The Accessibility Project:
http://a11yproject.com/posts/never-use-maximum-scale/
* Google:
https://developers.google.com/web/fundamentals/layouts/rwd-fundamentals/set-the-viewport?hl=en
@belbiy
Copy link

belbiy commented Aug 25, 2015

@chicgeek, thank you for your feedback!
In our Magento 2 themes we wanted to give more "app-like" experience to shoppers, and disabled the scaling. We believe that this is good UX for most users, since it eliminates unnecessary panning.

However, if you want to allow scaling in your theme, you can do this by using a layout update:

<page xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="../../../../../../../lib/internal/Magento/Framework/View/Layout/etc/page_configuration.xsd">
    <head>
        <meta name="viewport" content="width=device-width, initial-scale=1"/>
    </head>
</page>

in app\design\frontend\YourVendor\yourThemeName\Magento_Theme\layout\default_head_blocks.xml

You can read more about layout updates in our developer docs.

@blongden
Copy link

Inaccessible by default is not a good precedent. If Magento 2 is as successful as Magento 1 with themes based off this default one, there's going to be a lot of websites that cannot zoom just because it was not considered a requirement in the first place. This is bad for those that cannot read tiny fonts.

I'd like to see this patch accepted and a layout update provided for those that want to disable zoom (although this is such an anti-pattern it should be discouraged).

@cshaw-inviqa
Copy link

Hi,

I agree with @chicgeek here. Disabling a function of a browser that a user will expect to use is bad UX. It's on par with disabling a back button in the browser.

@markross
Copy link

Personally I think ignoring widely accepted best practices for accessibility is not a good approach. WCAG AA should be a minimum to aim for, otherwise you may be discriminating against disabled (in this case visually impaired) users, and can even potentially expose yourself to legal action (see https://econsultancy.com/blog/8834-rnib-gets-tough-with-bmibaby-over-accessibility/)

@erikhansen
Copy link
Contributor

@belbiy I strongly disagree with preventing zoom as a default configuration in Magento. It's a pretty bold move to presume to know more about usability than groups like W3C and Google. In one of the links that @chicgeek provided, Google has this to say about disabling zoom:

When set, these can disable the user’s ability to zoom the viewport, potentially causing accessibility issues.

Most touch browsers/devices do a great job of only zooming/panning when the user specifically intends to do so, so recreating the "mobile app" experience should not be a concern.

On pages like the product listing and product detail pages, allowing a user to pinch-to-zoom will allow them to see more details in the product images. This is especially true if a theme has implemented higher-resolution images using the <picture> tag, or a similar method.

@belbiy
Copy link

belbiy commented Aug 27, 2015

Everyone, thank you for the great feedback here. We definitely will consider this input next time we will re-evaluate the decision on zoom in our themes. For now, the decision has been made, but it doesn't mean it will stay that way.

@erikhansen provided a great example when the zoom feature can be really useful for users, and I look forward on implementing responsive images in our themes in similar way.

@erikhansen
Copy link
Contributor

@belbiy Ok, thanks for considering the dissenting opinions.

I hope that this PR gets merged in soon, as it seems like it is a clear decision. And the longer it stays as it is, the more likely it is that it will never get changed.

@alankent
Copy link

Feel free to ping in a week or two to make sure not forgotten. Lots of internal people (like me) are certainly watching this sort of feedback. I don't claim to be a UX expert, but trying to learn more internally.

@chicgeek
Copy link
Contributor Author

I hope that this PR gets merged in soon, as it seems like it is a clear decision. And the longer it stays as it is, the more likely it is that it will never get changed.

Agreed. Disabiling accessibility should not be the default - I think this is an easy win if it's merged sooner rather than later.

I'll ping again in a couple of weeks for reconsideration as requested. Thanks all.

@belbiy
Copy link

belbiy commented Aug 28, 2015

@chicgeek i have an update already. We decided to allow zoom in the Blank theme, and use XML layout update in Luma theme to disable zoom as i mentioned previously. That way we will keep the current UX for our demo theme, and use recommended viewport settings for a developer starter theme.

Again, thanks for the input everyone, keep it up!

Internal ticket MAGETWO-42173

@okorshenko
Copy link
Contributor

@chicgeek, Internal ticket MAGETWO-42173 is closed and delivered to mainline. Please, see 9c3403e
Please, let us know if you still have an issue

@okorshenko okorshenko closed this Sep 24, 2015
okorshenko pushed a commit that referenced this pull request Nov 16, 2017
Public Pull Requests

#12303 9764: exception message is wrong and misleading in findAccessorMethodName() of Magento\Framework\Reflection\NameFinder by @RomaKis

Fixed Public Issues

#9764 exception message is wrong and misleading in findAccessorMethodName() of Magento\Framework\Reflection\NameFinder
magento-devops-reposync-svc pushed a commit that referenced this pull request Sep 12, 2024
Merge 2.4-develop into AC-9712
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.

9 participants