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

Prevent automatic zoom when focusing inputs on iOS #8477

Merged
merged 1 commit into from
Nov 13, 2024

Conversation

Devessier
Copy link
Contributor

This is the result of a long discussion we had here: #8001.

The goal is to stop iOS from automatically zooming when the user focuses on an input whose font size is less than 16px.

The options were:

  1. Disable zoom for all devices
  2. Disable zoom for devices detected as iOS devices, which doesn't prevent users from zooming manually but fixes the auto-zoom bug
  3. Set the font size of the inputs to be equal to or greater than 16px—this change would take a lot of time

To me, the second option is the best, as iOS prevents developers from disabling zoom. They saw that it was overused and chose to restrict this setting. Setting a maximum-scale doesn't prevent users from zooming, but it fixes the initial bug we had.

My implementation can be seen as progressive enhancement: If we can detect that the user uses an iOS device, we'll set the maximum-scale viewport property. Relying on the user agent is always unstable, and the check might fail unpredictably. We might not disallow auto-zoom for some iOS devices.

However, I think we can either:

  • Invest some time to choose a more reliable user detection pattern if the one I suggest is not sufficient (we find many different checks on the internet, I'm not sure which one is the best)
  • Choose to apply the viewport setting to all devices and remove the JS code. According to my tests, it doesn't prevent zooming on desktops. However, it does on Android phones. I think it's not lovely to disallow zoom, but if the team agrees that we should go this way, I won't disagree.

I know my JavaScript code does not follow a pattern we want to spread in the app. The synchronous script will run as soon as possible to ensure the viewport is correctly set when the website launches. This shouldn't be an example followed by others.

Thanks, @harshit078, for your help in thinking about the best option.

I'm tagging @lucasbordeau and @charlesBochet for a technical review.

I would appreciate if someone could test on a more recent iOS device than mine.

Here is a demonstration of the behavior on iOS:

RPReplay_Final1731503005.4.MP4

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Summary

This PR implements a solution to prevent automatic zooming on iOS devices when users focus on input fields with font sizes smaller than 16px, while maintaining manual zoom functionality.

  • Added inline JavaScript in /packages/twenty-front/index.html to detect iOS devices and set maximum-scale=1.0 viewport property
  • Moved viewport meta tag placement for optimal timing of zoom prevention
  • Solution preserves manual zoom capability while preventing disruptive auto-zoom behavior
  • Implementation uses progressive enhancement approach, only applying changes to detected iOS devices
  • Addresses reported UX issues with input fields, dropdowns, and page scrolling on iOS devices

1 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile

}
}

const isIOS = /iPad|iPhone/.test(navigator.userAgent);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: User agent detection can be unreliable. Consider using feature detection or multiple iOS detection methods for better reliability.

Copy link
Member

@FelixMalfait FelixMalfait left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!
Just to recap for to leave the full conclusion in one place, in my opinion disabling zoom on Android is a better option from a product/design perspective since our business goal is to have an app-like behavior (it's also better from a code perspective imo since it's just a one-line change), but we've spent more than enough time debating this - let's merge this.

@FelixMalfait FelixMalfait merged commit 898006f into main Nov 13, 2024
19 checks passed
@FelixMalfait FelixMalfait deleted the prevent-automatic-ios-zoom-focus-input branch November 13, 2024 14:34
Copy link

Fails
🚫

node failed.

Log

�[31mError: �[39m RequestError [HttpError]: You have exceeded a secondary rate limit. Please wait a few minutes before you try again. If you reach out to GitHub Support for help, please include the request ID C4C7:168964:5E68E42:B8C1733:6734B925.
    at /home/runner/work/twenty/twenty/node_modules/�[4m@octokit�[24m/request/dist-node/index.js:86:21
�[90m    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)�[39m {
  status: �[33m403�[39m,
  response: {
    url: �[32m'https://api.github.com/search/issues?q=is%3Apr%20author%3ADevessier%20is%3Aclosed%20repo%3Atwentyhq%2Ftwenty&per_page=2&page=1'�[39m,
    status: �[33m403�[39m,
    headers: {
      �[32m'access-control-allow-origin'�[39m: �[32m'*'�[39m,
      �[32m'access-control-expose-headers'�[39m: �[32m'ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Used, X-RateLimit-Resource, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type, X-GitHub-SSO, X-GitHub-Request-Id, Deprecation, Sunset'�[39m,
      connection: �[32m'close'�[39m,
      �[32m'content-encoding'�[39m: �[32m'gzip'�[39m,
      �[32m'content-security-policy'�[39m: �[32m"default-src 'none'"�[39m,
      �[32m'content-type'�[39m: �[32m'application/json; charset=utf-8'�[39m,
      date: �[32m'Wed, 13 Nov 2024 14:35:18 GMT'�[39m,
      �[32m'referrer-policy'�[39m: �[32m'origin-when-cross-origin, strict-origin-when-cross-origin'�[39m,
      server: �[32m'github.com'�[39m,
      �[32m'strict-transport-security'�[39m: �[32m'max-age=31536000; includeSubdomains; preload'�[39m,
      �[32m'transfer-encoding'�[39m: �[32m'chunked'�[39m,
      vary: �[32m'Accept-Encoding, Accept, X-Requested-With'�[39m,
      �[32m'x-content-type-options'�[39m: �[32m'nosniff'�[39m,
      �[32m'x-frame-options'�[39m: �[32m'deny'�[39m,
      �[32m'x-github-api-version-selected'�[39m: �[32m'2022-11-28'�[39m,
      �[32m'x-github-media-type'�[39m: �[32m'github.v3; format=json'�[39m,
      �[32m'x-github-request-id'�[39m: �[32m'C4C7:168964:5E68E42:B8C1733:6734B925'�[39m,
      �[32m'x-ratelimit-limit'�[39m: �[32m'30'�[39m,
      �[32m'x-ratelimit-remaining'�[39m: �[32m'30'�[39m,
      �[32m'x-ratelimit-reset'�[39m: �[32m'1731508577'�[39m,
      �[32m'x-ratelimit-resource'�[39m: �[32m'search'�[39m,
      �[32m'x-ratelimit-used'�[39m: �[32m'1'�[39m,
      �[32m'x-xss-protection'�[39m: �[32m'0'�[39m
    },
    data: {
      documentation_url: �[32m'https://docs.github.com/free-pro-team@latest/rest/overview/rate-limits-for-the-rest-api#about-secondary-rate-limits'�[39m,
      message: �[32m'You have exceeded a secondary rate limit. Please wait a few minutes before you try again. If you reach out to GitHub Support for help, please include the request ID C4C7:168964:5E68E42:B8C1733:6734B925.'�[39m
    }
  },
  request: {
    method: �[32m'GET'�[39m,
    url: �[32m'https://api.github.com/search/issues?q=is%3Apr%20author%3ADevessier%20is%3Aclosed%20repo%3Atwentyhq%2Ftwenty&per_page=2&page=1'�[39m,
    headers: {
      accept: �[32m'application/vnd.github.v3+json'�[39m,
      �[32m'user-agent'�[39m: �[32m'octokit-rest.js/18.12.0 octokit-core.js/3.6.0 Node.js/18.20.4 (linux; x64)'�[39m,
      authorization: �[32m'token [REDACTED]'�[39m
    },
    request: { hook: �[36m[Function: bound bound register]�[39m }
  }
}
danger-results://tmp/danger-results-e5763365.json

Generated by 🚫 dangerJS against b8571f8

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.

2 participants