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 fullscreen dialog for small devices #12

Open
emibcn opened this issue Oct 8, 2020 · 8 comments
Open

Fix fullscreen dialog for small devices #12

emibcn opened this issue Oct 8, 2020 · 8 comments
Assignees
Labels
bug Something isn't working design Similar to UI/UX Hacktoberfest Simple issues suitable Hacktoberfest PRs javascript Javascript knowledge needed material/ui Related to Material/UI dependency react React knowledge needed ui/ux Issues related to design/UI/UX

Comments

@emibcn
Copy link
Owner

emibcn commented Oct 8, 2020

I have been reported about a bug in some small devices, where the dialog' Close button falls out of the view. The user needs to scroll down a bit to see the button. Looks like it's when the app is viewed directly from browser, while it is showing the navigation bar.

Initial view (before scrolling):

image

Final view (after scrolling down):

image

Fle: https://github.com/emibcn/covid/blob/master/app/src/Widget/Actions.jsx#L53

Some extra info: https://stackoverflow.com/questions/37395561/how-to-hide-a-mobile-browsers-address-bar
Possible solution: https://chanind.github.io/javascript/2019/09/28/avoid-100vh-on-mobile-web.html

@emibcn emibcn added bug Something isn't working Hacktoberfest Simple issues suitable Hacktoberfest PRs react React knowledge needed javascript Javascript knowledge needed ui/ux Issues related to design/UI/UX design Similar to UI/UX material/ui Related to Material/UI dependency labels Oct 8, 2020
@ashish979
Copy link
Contributor

@emibcn I can fix this one too if you want?

@emibcn
Copy link
Owner Author

emibcn commented Oct 9, 2020

@ashish979 Its yours!

@ashish979
Copy link
Contributor

Hi @emibcn
I have raised PR for this fix #27
I verified the fix on my android device (chrome browser) but please check once on other mobile browser too and let me know if the fix is ok.

ashish979 added a commit to ashish979/covid that referenced this issue Oct 13, 2020
@ashish979
Copy link
Contributor

Hi @emibcn
Sorry for the delay, been busy with some important tasks.

I tried replicating the issue on firefox browser on an android device but could not successfully replicate it. Can you please help me with the device version and browser version so that I can check if the issue still exists?
Below is the screenshot from my device:
image

Also I have raise PR #31 as per your suggestion adding paperFullScreen. Please test if this fixes the issue.

@emibcn
Copy link
Owner Author

emibcn commented Oct 13, 2020

Hi @ashish979 ,

Note: You don't need to remove and recreate a PR: just push new commits to the branch you were working on and them will be visible in the PR while it is open.

Also I have raise PR #31 as per your suggestion adding paperFullScreen. Please test if this fixes the issue.

About the fix you provided in #31 : paperFullScreen property is not intended to be used like that (did you read the docs provided in the comment #27 (comment) ?).

@emibcn
Copy link
Owner Author

emibcn commented Oct 13, 2020

I tried replicating the issue on firefox browser on an android device but could not successfully replicate it. Can you please help me with the device version and browser version so that I can check if the issue still exists?

I use DuckDuckGo browser app, which I thought it was a fork of Firefox, but now I see it might not be like that...

@ashish979
Copy link
Contributor

About the fix you provided in #31 : paperFullScreen property is not intended to be used like that (did you read the docs provided in the comment #27 (comment) ?).

Hi @emibcn
I did read the docs and used the property as mentioned. paperFullScreen is adding some styles when the fullScreen property is true.

According to the docs:
image

And in the actual implementation:
image

I can see these styles applied in my browser:
image

Can't say what I am missing here. Please suggest.

@emibcn
Copy link
Owner Author

emibcn commented Oct 25, 2020

Sorry about the delay.

In the current implementation (without your commit), same classes and CSS rules are applied:
image
image

I think this is a problem that needs a hacky solution, probably better suited into MaterialUI libs (or even at DuckDcukGo!), which would need to detect the browser and apply hack solution on buggy browsers. I've just opened an issue there about this problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working design Similar to UI/UX Hacktoberfest Simple issues suitable Hacktoberfest PRs javascript Javascript knowledge needed material/ui Related to Material/UI dependency react React knowledge needed ui/ux Issues related to design/UI/UX
Projects
None yet
Development

No branches or pull requests

2 participants