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

Add PinchToZoom option #2252

Merged
merged 5 commits into from Feb 17, 2022
Merged

Add PinchToZoom option #2252

merged 5 commits into from Feb 17, 2022

Conversation

micronen
Copy link
Contributor

@micronen micronen commented Feb 9, 2022

Allow enabling PinchToZoom in WebView via option in preferences

Summary

Allow enabling PinchToZoom in WebView via option in preferences.
It allows, for e.g., using a picture ui with a lot of entities on a small screen phone.
This was discused is several threads:
https://community.home-assistant.io/t/ha-web-interface-enable-pinch-to-zoom-on-mobile-devices/30389
https://community.home-assistant.io/t/pinch-zoom-on-mobile-idea/104153
https://community.home-assistant.io/t/zoom-inn-android-app/353758
and maybe some more...

Screenshots

Screenshot_20220209-194500_Home Assistant
Screenshot_20220209-194515_Home Assistant
Screenshot_20220209-194530_Home Assistant
Screenshot_20220209-194102_Home Assistant
Screenshot_20220209-193945_Home Assistant
Screenshot_20220209-194022_Home Assistant

Link to pull request in Documentation repository

Documentation: home-assistant/companion.home-assistant#683

Any other notes

This is my 1st try at kotlin and/or android app. Please give me feedback if I made some rooky mistakes.

Allow enabling PinchToZoom in WebView via option in preferences
Copy link
Member

@dshokouhi dshokouhi left a comment

Choose a reason for hiding this comment

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

Thanks for the PR 🙏

The icon does not need to be changed, was more of a suggestion. We should make sure to remove the duplicate code as it is unnecessary in the first location.

app/src/main/res/xml/preferences.xml Outdated Show resolved Hide resolved
@dshokouhi
Copy link
Member

So I am looking at the debug APK but I don't see where this setting is working. Does this only work on certain cards? I have a zwave map but thats a custom card and pinch to zoom on it works in the production version of the app so I don't think it has any impact there. Maybe I am missing something here?

@micronen
Copy link
Contributor Author

On my phone it works for the whole app, menus and everything. You turn it on and can pinch anything.
I have pinch enabled for browsers. Maybe it is a requirement for it to work in the app?
Can you please, try the following from https://community.home-assistant.io/t/ha-web-interface-enable-pinch-to-zoom-on-mobile-devices/30389/17`

Put this in a file called zoom-enable.js in your config/www folder

document.querySelector("meta[name=viewport]").setAttribute("content", "width=device-width, viewport-fit=cover");
Add this to your configuration.yaml:

frontend:
extra_module_url:
- /local/zoom-enable.js

@dshokouhi
Copy link
Member

Can you please, try the following from https://community.home-assistant.io/t/ha-web-interface-enable-pinch-to-zoom-on-mobile-devices/30389/17`\n\nPut this in a file called zoom-enable.js in your config/www folder\n\ndocument.querySelector("meta[name=viewport]").setAttribute("content", "width=device-width, viewport-fit=cover");\nAdd this to your configuration.yaml:\n\nfrontend:\nextra_module_url:\n- /local/zoom-enable.js

If this is required for this feature then we shouldn't move forward with it in the app. We can't ask users to add custom stuff like that.

@jpelgrom
Copy link
Member

If this is required for this feature then we shouldn't move forward with it in the app

I agree, but: iOS has an option to enable pinch to zoom and will adjust the viewport meta tag on load to make it work when turned on, see home-assistant/iOS#1472. (That PR also links to frontend issues about enabling/disabling zoom by default.) It is a bit of a hack but personally I think that if it was implemented in the same way in this PR, it should be acceptable.

@dshokouhi
Copy link
Member

If this is required for this feature then we shouldn't move forward with it in the app

I agree, but: iOS has an option to enable pinch to zoom and will adjust the viewport meta tag on load to make it work when turned on, see home-assistant/iOS#1472. (That PR also links to frontend issues about enabling/disabling zoom by default.) It is a bit of a hack but personally I think that if it was implemented in the same way in this PR, it should be acceptable.

I agree having the app perform it is more desirable in this situation. I think it's ok to match iOS behavior here.

@micronen
Copy link
Contributor Author

I've added a JS query to override the content attribute in onPageFinished

webView.evaluateJavascript(
    "document.querySelector(\"meta[name=viewport]\").setAttribute(\"content\", \"width=device-width,user-scalable=yes,viewport-fit=cover,initial-scale=1\");"
) {}

Please try it again.

BTW, in the chrome browser you can use Settings -> Accessibility -> Force enable zoom, to make it disregard user-scalable=no

BTW2, why HASS even have this set to no in the first place?

@SkechyWolf
Copy link
Contributor

SkechyWolf commented Feb 12, 2022

Please try it again.

Works for me now

@dshokouhi
Copy link
Member

Please try it again.

latest commit works for me too, thank you :)

BTW2, why HASS even have this set to no in the first place?

I think this PR will help answer this question :) home-assistant/frontend#8353

Update on two events:
1. onPageFinished for 1st time load
2. onResume to update on setting change
@micronen
Copy link
Contributor Author

Please review & check latest version

Copy link
Member

@dshokouhi dshokouhi left a comment

Choose a reason for hiding this comment

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

Tested debug APK and it works for me

@JBassett JBassett merged commit 7eced55 into home-assistant:master Feb 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants