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

fix(svg-mask): consider devices with on-screen buttons #15

Merged
merged 1 commit into from
Nov 25, 2020

Conversation

Gabrielfcs
Copy link
Contributor

Replaces the use of the window size with the screen size, to ensure that on Android devices (that the gesture option is enabled, that is, that it does not use buttons on the screen) the mask is correctly sized.

As it is today, on Android devices that disable the on-screen buttons on:
PHOTO-2020-09-30-17-23-55

replaces the use of the window size with the screen size, to ensure that on Android devices (that the gesture option is enabled, that is, that it does not use buttons on the screen) the mask is correctly sized
@xcarpentier
Copy link
Owner

Hi,
Thanks!
But not sure that is needed, you can play with a prop to achieve this kind of thing, maybe documentation is not explicit enough.

@Gabrielfcs
Copy link
Contributor Author

Hi @xcarpentier, I may not have expressed myself properly and you may not have understood the problem I reported, so I will try to describe the steps I took and the solution I found.

After searching all the documentation, I ended up not finding anything that could solve this in Androids that have this option, to remove the buttons and keep only the gestures (functionality similar to the action bar at the bottom of the iphones screen).

So I started to search the code and even then I didn't find any option that resizes the size of the base SVG (the svg that fills the entire screen, which is cut by the mask).

So that the space where the buttons used to be can be filled, we need this switch from window to screen, as this is a failure of React Native in recognizing the screen without the buttons. That continues to consider the buttons as an unavailable part of the screen (since they are no longer visible).

I do not see any problems in making this exchange, as I ended up using it in more than one project and all I used were large projects that had no side effects from this change.

Sorry my bad English 😅

@ipakhomov
Copy link

@xcarpentier hello and thank you for your work!

I faced the same issue for some Android devices and switching to screen height instead of window height solves it.

I think that the suggestion of @Gabrielfcs makes sense - from the docs:

For Android the window dimension will exclude the size used by the status bar (if not translucent) and bottom navigation bar

There are some issues about the window height unreliability:

So usage of screen dimension instead of window looks more reliable.
To avoid breaking changes this might be added to TourGuideProviderProps to allow configure this behavior.

Thoughts?

@xcarpentier xcarpentier merged commit 77ef91f into xcarpentier:master Nov 25, 2020
@ebouJ
Copy link

ebouJ commented Nov 29, 2020

Did you release this @xcarpentier ?

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.

4 participants