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 #12 #27

Closed
wants to merge 1 commit into from
Closed

Fix #12 #27

wants to merge 1 commit into from

Conversation

ashish979
Copy link
Contributor

Copy link
Owner

@emibcn emibcn left a comment

Choose a reason for hiding this comment

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

It does not looks very good, sorry:

My mobile works well with the current production version, so this needs a hack only for buggy devices.
I suppose the hack has to be applied:

  • Only when the fullscreen variable is true
  • Each time the dialog is opened. It means, recalculate and reapply the style each time it opens. For this, it should use a special form of makeStyle based on props
  • Only when an 100hv element's height is higher than window.innerHeight (maybe this condition is not necessary)

Does it makes sense or am I missing something?

@emibcn
Copy link
Owner

emibcn commented Oct 10, 2020

Investigating more, I've found:

  • The buggy one is not the device, but the browser: Firefox based ones got this error
  • Tried with that the version proposed in this PR and, effectively, it's not solving the problem
  • Maybe it can be easily solved using paperFullScreen property from Material UI Dialog

@ashish979
Copy link
Contributor Author

ashish979 commented Oct 12, 2020

Investigating more, I've found:

  • The buggy one is not the device, but the browser: Firefox based ones got this error
  • Tried with that the version proposed in this PR and, effectively, it's not solving the problem
  • Maybe it can be easily solved using paperFullScreen property from Material UI Dialog

Thanks for the input. I will do a thorough testing and raise another PR.

@ashish979 ashish979 closed this Oct 12, 2020
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