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

Memorize responsive seems broken on phones #106

Closed
mikklfr opened this issue Mar 9, 2017 · 19 comments
Closed

Memorize responsive seems broken on phones #106

mikklfr opened this issue Mar 9, 2017 · 19 comments
Labels
bug to be release Fixed, to be release
Milestone

Comments

@mikklfr
Copy link
Collaborator

mikklfr commented Mar 9, 2017

The paint activity button seems to have troubles with small screen.
Some CSS should be able to fix it.

@dinesh-sirvi
Copy link
Contributor

Couldnt able to reproduce the issue, Can u elaborate it more or provide me the steps to reproduce it?

@mikklfr
Copy link
Collaborator Author

mikklfr commented Mar 12, 2017

My bad, it's the memorize activity. I'm updating the issue.

You can see the quit button going out of the toolbar on this photo.

https://www.facebook.com/photo.php?fbid=10154521813268510&set=pcb.10154521813863510&type=3&theater

@mikklfr mikklfr changed the title Paint buttons responsive seems broken on phones Memorize buttons responsive seems broken on phones Mar 12, 2017
@mikklfr mikklfr changed the title Memorize buttons responsive seems broken on phones Memorize responsive seems broken on phones Mar 12, 2017
@dinesh-sirvi
Copy link
Contributor

I'll Check it....

@rajeevravindran
Copy link
Contributor

Can't reproduce the problem. @mikklfr Can you give the viewport dimensions of the browser on your phone?

@mikklfr
Copy link
Collaborator Author

mikklfr commented Mar 13, 2017

I've been able to reproduce using the Galaxy S5 viewport in Google Chrome debug. 640 x 360

@rajeevravindran
Copy link
Contributor

image

.toolbar{ padding : 0 50px; }

This solves it. Creating a PR

rajeevravindran added a commit to rajeevravindran/sugarizer that referenced this issue Mar 13, 2017
Reduced the padding of .toolbar to accomadate the close button.
@mikklfr
Copy link
Collaborator Author

mikklfr commented Mar 13, 2017

Better, popular phones are working now but I still have issues using
590px <= width <= 638px

@rajeevravindran
Copy link
Contributor

rajeevravindran commented Mar 13, 2017

Yes. Maybe a css media query for this range? 570px - 638px

@rajeevravindran
Copy link
Contributor

image
Seems to be working. I'll make a PR?
@media screen and (min-width: 590px) and (max-width: 638px){ .toolbar{ padding: 0 25px; } }

@llaske llaske added the bug label Dec 3, 2017
@utkarsh-raj
Copy link
Contributor

Some insight -

  • The responsiveness is broken across several activities (including Memorize, the original activity in question)

  • In some other activities, the Stop button simply goes away on some viewports, which can be verified by the Google Chrome Developer Tools

  • The issue is also there for several modern devices like the iPhone X

Probably some CSS for modern viewport breakpoints should be able to fix this. But the important point is that the issue has a much broader scope than only Memorize. I am currently working on this and some reviews will be great :)

screenshot from 2019-02-21 23-53-25
screenshot from 2019-02-21 23-52-04

@llaske
Copy link
Owner

llaske commented Feb 21, 2019

Here's few insights regarding Sugarizer responsive policy:

  1. The preferred way to run Sugarizer on mobile devices is via the iOS and Android app.
  2. Sugarizer UI is adapted to screen larger than longer (landscape), this rule is forced on iOS and Android apps. In browser in portrait mode (like in your screen capture), nothing is required from activities today.
  3. Sugarizer is not adapted today to smartphone, it's not only related to toolbar but also to the size of graphical UI items in activities (too small to be handle with fingers). Sugarizer activities running on smartphone should do the best to run: minimal behavior is to be able to be launched and stopped. Nothing more is required. And of course, due to 1., this requirement is in landscape mode.

@utkarsh-raj
Copy link
Contributor

Many thanks for the insights @llaske , I missed the point that the Android and iOS Application are a better way for Sugarizer on mobile devices. This basically means that the disappearing stop button is not an issue for small viewports.

So maybe the issue can be safely closed ?
Or the CSS breakpoints should be edited for the Memorize Activity ? (and other related activities ?)

@llaske
Copy link
Owner

llaske commented Feb 23, 2019

@utkarsh-raj it should not be closed if the issue concern the Android version and forbid Memorize to follow the minimal behavior: to be able to be launched and stopped.

@utkarsh-raj
Copy link
Contributor

From what I could test, the Android version works fine for standard viewports and the minimal expected behaviour is satisfied. Any specific viewport resolution @llaske ?

@llaske
Copy link
Owner

llaske commented Feb 23, 2019

Sugarizer Core was thought to be responsive until 480x320 because it was the resolution of the Firefox OS Keon phone. But this resolution is not supported by all activities. Plus I don't think this resolution make sense today. So the minimum should probably be twice this resolution.

@utkarsh-raj
Copy link
Contributor

All right I will get to work on this. A bit of research on my part is required. Thanks for the pointers @llaske :)

@utkarsh-raj
Copy link
Contributor

utkarsh-raj commented Feb 24, 2019

#282 (comment)

The required file is identified already by @rajeevravindran .I have verified and implemented the required Media Queries. The PR now also fixes the responsiveness issues for all major viewports.

@llaske
Copy link
Owner

llaske commented Feb 25, 2019

Fixed in #282

@llaske
Copy link
Owner

llaske commented Sep 26, 2019

Closed in dd3c58d

@llaske llaske closed this as completed Sep 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug to be release Fixed, to be release
Projects
None yet
Development

No branches or pull requests

5 participants