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

Update responsive view #83

Closed
wants to merge 3 commits into from

Conversation

berdosi
Copy link
Contributor

@berdosi berdosi commented Jan 4, 2020

Moving discussion here from #34 (Smartphone view).

Current status:

  • city list is on the bottom, horizontally scrollable (by removing flex-wrap: wrap;)
    • this solves the issue of longer city lists blocking the useful content as per
  • add city item has 100% width, but it looks odd when it is only partially visible, and the form is open (screenshot below).
    • how about hiding "Add city" altogether?
    • we may be out of luck with pure CSS: as it is in the same LI as the cities, I not sure we can wrap it into the next row.
  • tablet mode still not addressed

pngout-fs8-nq8

Signed-off-by: Balint Erdosi <[email protected]>
Signed-off-by: Balint Erdosi <[email protected]>
@e-alfred
Copy link
Collaborator

I am not sure how many people want to add cities on a smartphone. Anyway, they can switch to the desktop view in most mobile browsers so it can be used with an additional tap, therefore I think it is not completely necessary to have it on the mobile view (where space is quite limited anyway).

@nerzhul
Copy link
Collaborator

nerzhul commented Jan 13, 2020

it can be nice to have a better UI on mobile, some likes to have it, maybe we can be responsive :)

@berdosi
Copy link
Contributor Author

berdosi commented Jan 13, 2020

I can imagine the use case for adding cities on the mobile interface (e.g. just having arrived to a new city for a few days), as such I would rather risk making it look odd in some edge cases.

I wouldn't be hard to make it look nice by modifying the template - however, as long as I'm toying with #80 (Vue migration), I am somewhat motivated to avoid modifying it… :)

@e-alfred
Copy link
Collaborator

use case for adding cities on the mobile interface (e.g. just having arrived to a new city for a few days)

Yes, this sounds like a viable use case, we should leave it in place for now. Another idea for this use case would be to use the geolocation feature of most browsers to automatically find the location based on GPS coordinates.

@berdosi
Copy link
Contributor Author

berdosi commented Jan 25, 2020

I improved a bit on the experience at tablet sizes: the city list jumps earlier to the bottom, and there are some further breakpoints for wider screens, so that more than two cities fit on the horizontal list at a time. Tablet sizes could accommodate a two-row layout, but it would be quite a hassle to rewrite it.

Adding geolocation is more complex: OpenWeatherMap takes coordinates into a different parameter than it takes the city name to (?q=cityName vs ?lat=0.00&lon=0.00), either WeatherController.php is to be extended, or the location name would need to be looked up.

@berdosi berdosi marked this pull request as ready for review January 25, 2020 21:52
@e-alfred e-alfred self-requested a review March 1, 2020 12:54
@e-alfred
Copy link
Collaborator

I added some additional weather information now provided by the OpenWeatherMap API, but currently the UI isn't that pretty:

image

Do you have a good idea how we could structure the city pane especially? I think it should be on the top rather than the left side and using the space more efficiently, while the forecast data should be below just sticking to the left at all times.

@e-alfred
Copy link
Collaborator

I fixed up the desktop view, @berdosi could you take a look if it works with your PR as well:

image

@berdosi
Copy link
Contributor Author

berdosi commented Apr 27, 2020

Thanks, I'm a fan of the new layout. :) Also, apologies for the late reply.

I'd say this PR doesn't break it, and it is some improvement over how the menu is now.
There is still some horizontal scrolling on the forecast table, but there's always been.

I see quite some improvement areas for this PR, though:

  • The forecast table can probably be reflown at narrow screens instead of (or apart from) just dropping columns
  • More importantly, I've found how the navigation should be done as per the guidelines, which got me disillusioned about using my bespoke carefully hand-adjusted breakpoints-based approach.
    • Moving to this one is a bigger piece of work, though.

@FadeFx
Copy link

FadeFx commented Jun 14, 2020

i dont get it why weather would not follow the usual design standards and in mobile view hide the left panel and display a burger icon instead to temprarily unhide it... is there a special reason for that?

@FadeFx
Copy link

FadeFx commented Sep 4, 2020

Too bad, not much talk here... is that app dead?

@nerzhul
Copy link
Collaborator

nerzhul commented Sep 16, 2020

mostly not many developers on this one, the API is stable, only design has to be fixed. This app is maintained by 1 person only

@e-alfred
Copy link
Collaborator

@berdosi Is this superseded by #92?

@berdosi
Copy link
Contributor Author

berdosi commented Dec 28, 2020

@e-alfred yes.
#92 uses stock Nextcloud components, hence it follows the design language better.
This one makes use of the current frontend code, and patches it up with CSS.

@e-alfred
Copy link
Collaborator

@berdosi I will close this then and hopefully we can merge #92 soon.

@e-alfred e-alfred closed this Dec 28, 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.

4 participants