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

[mobile] Add meta viewport tag. #6967

Merged
merged 1 commit into from
Jan 29, 2020
Merged

Conversation

nisarhassan12
Copy link
Contributor

Hi!
This Pr addresses the following from #3557

The page is very zoomed in because there is no meta-tag controlling the viewport initials size (https://developer.mozilla.org/en-US/docs/Mozilla/Mobile/Viewport_meta_tag)

Co-Authored-By: Eric Rabil <[email protected]>

Signed-off-by: Nisar Hassan Naqvi <[email protected]>
@jankeromnes
Copy link
Member

Hi @nisarhassan12, many thanks for catching this! The missing viewport tag is likely a root cause for many of the strange zooming problems we see on mobile.

I will try to test this on my Pixel 3, and compare the UX with/without your fix. If this works as expected, it would be a big improvement for Theia's experience on mobile. 👍

@jankeromnes jankeromnes changed the title add meta viewport tag. [mobile] Add meta viewport tag. Jan 27, 2020
@akosyakov akosyakov added the ipad issues related to iPad label Jan 27, 2020
@akosyakov
Copy link
Member

btw we have similar settings on webviews copied from VS Code:

<meta name="viewport"
content="width=device-width, initial-scale=1.0, maximum-scale=1.0, minimum-scale=1.0, user-scalable=no">

maybe something useful as well

@vince-fugnitto
Copy link
Member

@nisarhassan12 please be sure to accept the Eclipse Contributor Agreement (ECA) so that we can accept the contribution :)

@nisarhassan12
Copy link
Contributor Author

@nisarhassan12 please be sure to accept the Eclipse Contributor Agreement (ECA) so that we can accept the contribution :)

Thanks @vince-fugnitto I have accepted the ECA 🙂 .

@vince-fugnitto
Copy link
Member

@nisarhassan12 please be sure to accept the Eclipse Contributor Agreement (ECA) so that we can accept the contribution :)

Thanks @vince-fugnitto I have accepted the ECA .

I'll let @jankeromnes complete the review, I don't have an iPad (or large touch device) to test the pull-request against and I trust his judgement :)

@nisarhassan12
Copy link
Contributor Author

nisarhassan12 commented Jan 28, 2020

I'll let @jankeromnes complete the review, I don't have an iPad (or large touch device) to test the pull-request against and I trust his judgement :)

Ok. Thanks! :)

Copy link
Member

@jankeromnes jankeromnes left a comment

Choose a reason for hiding this comment

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

Super-cool fix, many thanks @nisarhassan12! 💯

I think this doesn't change the behavior on larger screens, but on smaller screens like smartphones this is a revolution -- Theia goes from broken to usable.

Also this is recommended by Lighthouse as a best practice.

Before

Theia starts out very un-zoomed, and as soon as you tap anywhere, the browser zooms you in a lot on a specific spot, and doesn't allow you to control zoom or pan around (probably because we cancel touchmove events). This is almost not usable.

Screenshot 2020-01-29 at 08 30 42 Screenshot_20200129-082426

Now

Theia starts at a comfortable zoom level, where you can read and control almost the entire IDE. I agree it looks a bit awkward because it's so narrow, but I can use this already. Now we'll "just" need to fix other small usability issues, like resizing panels on mobile, and we'll have a productive coding environment on smartphones.

Screenshot_20200129-082409

@akosyakov akosyakov merged commit 3ad2b62 into eclipse-theia:master Jan 29, 2020
@jankeromnes
Copy link
Member

Also, thanks a lot @akosyakov and @vince-fugnitto for chiming in! 👍

@jankeromnes
Copy link
Member

And of course congratulations @nisarhassan12 on your first commit in Theia! 🎉🥇

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

Successfully merging this pull request may close these issues.

4 participants