Skip to content
This repository was archived by the owner on Sep 27, 2022. It is now read-only.

[MAINTAIN-200] remove buttons margin for non IOS devices. #110

Merged
merged 3 commits into from
Nov 12, 2021

Conversation

manachynskyi
Copy link
Contributor

Issue This PR should improve indents between buttons and the bottom of the screen on iOs/Android devices.

This PR should be compared on mobile iOS and Android (mobile resolution of desktop browser)
Steps for review (on mobile):

@manachynskyi manachynskyi requested a review from hamrant November 11, 2021 10:30
@hamrant hamrant requested review from duozersk and podarok November 11, 2021 10:37
Copy link
Contributor

@podarok podarok left a comment

Choose a reason for hiding this comment

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

Your thoughts?

@@ -12,6 +12,7 @@
"bootstrap": "^4.6.0",
"bootstrap-vue": "^2.21.2",
"core-js": "^3.18.3",
"mobile-device-detect": "^0.4.3",
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we sure we want to have this library https://github.com/duskload/mobile-device-detect/pulls
A lot of opened security issues @hamrant

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rather fork it or get maintainer access and fix those security issues

Copy link
Contributor

@hamrant hamrant Nov 11, 2021

Choose a reason for hiding this comment

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

@podarok nice catch!

Ok, we can try a solution without an additional library with data from Window.navigator - https://developer.mozilla.org/en-US/docs/Web/API/Window/navigator (For now it has good browser compatibility).

How to detect iOs device - https://racase.com.np/javascript-how-to-detect-if-device-is-ios/
How to detect mobile device - https://stackoverflow.com/questions/11381673/detecting-a-mobile-browser

const isMobile = navigator.userAgentData.mobile; //resolves true/false

cc @manachynskyi

Copy link
Contributor

Choose a reason for hiding this comment

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

Great, let's use it instead of insecure dependency
Thank you @hamrant

@manachynskyi
Copy link
Contributor Author

I used only checking by userAgent, because userAgentData is experimental and not supported by all browsers. I tested on different iphones and ipads (on emulator) it seems that all works fine.

@hamrant hamrant requested a review from podarok November 12, 2021 09:31
@podarok podarok merged commit b69ff95 into ymcatwincities:4.x Nov 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants