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

#44 mobile layout #74

Merged
merged 27 commits into from
May 20, 2021
Merged

#44 mobile layout #74

merged 27 commits into from
May 20, 2021

Conversation

Janekdererste
Copy link
Collaborator

@Janekdererste Janekdererste commented May 14, 2021

This PR attempts to solve #44 which is supposed to create a mobile friendly layout. The current status should be available here:

http://gh-maps-react.s3-website.eu-central-1.amazonaws.com/

@karussell and @easbar please have a look at the current version to provide some feedback. I assembled a list of things that need to be done before we can mark the issue as finished. Feel free to add items to that list by editing this comment if that's possible

What is still missing:

  • Context menu for touch input in the map panel
  • Selection of alternative routes. Currently one can select an alternative route by touching on the line displayed on the map. This is very hard to achieve though since the touch area is quite small.
  • the placeholder for alternative routes is still 3 routes even on mobile where only one is shown
  • Button styling for close button on full screen text search. Add "Esc"-button to leave this view too.
  • clear geocoding results when input loses focus
  • flip chevron of Query result to point upwards - maybe replace with something else entirely?
  • Show collapsed search view only if a route is available
  • attribution to OpenStreetMap etc needs to be done in mobile too. Add them to "Powered by" panel?
  • link should be www.graphhopper.com (currently it is just graphhopper.com)
  • scrolling instructions on mobile is somehow not possible in firefox can't reproduce
  • selecting a different map on mobile nearly always results in selecting kurviger first before I can open it
  • collapsed search view breaks layout on firefox mobile. See comment and images below
  • graphhopper logo takes up too much space
  • Fix overflowing powered by container when autocomplete box is expanded

@karussell
Copy link
Member

Thanks a lot! This is yet another nice update :)

This collapsing is my favorite feature as it is a phenomenal simple and nice solution for the space problem on mobile! And especially useful for cases with many via points. Also the alternative route selection is nice already.

Have added some things to your list. But maybe we separate some of them from this PR to merge it faster.

flip chevron of Query result to point upwards - maybe replace with something else entirely?

What do you mean here?

clear geocoding results when input loses focus

I think, this is a bit related to #54.

@Janekdererste
Copy link
Collaborator Author

flip chevron of Query result to point upwards - maybe replace with something else entirely?

The icon of the button which expands a routing result is called chevron. It points downwards. But the result opens from the bottom to the top on small screens. So maybe it should point upwards then. Not the most important point I guess.

@easbar
Copy link
Member

easbar commented May 16, 2021

This is very nice, thanks a lot 👍

I noticed an issue on Android (Firefox 68.2.0, Android 10): Before the route is calculated the input box text never exceeds the screen width. But once the route is shown the location text takes up more space and we have to scroll the screen to see all the text and there is white space to the right of the map. Chrome works on the same phone.

search view result view
image image

I can also reproduce this using iPhone 5s in the Firefox dev tools:

image

But strangely, when I actually open the website on my iPhone 5s it works fine. However, on my phone I see another minor issue: The layer icon is not located in the corner of the map (in both Firefox and Safari)

image

Another minor thing: The 'Powered by GraphHopper` logo still kind of takes a lot of space and since the url is (or will be) graphhopper.com/maps I am not sure if this is really necessary.

scrolling instructions on mobile is somehow not possible in firefox

This works on my iPhone.

@Janekdererste
Copy link
Collaborator Author

Also the alternative route selection is nice already.

This is good to hear. I'll leave it like this then? I have no good idea yet how to implement a selection on the routeing result list level.

…een small and expanded search view

* Reduce padding aroung graphhopper logo on small screens
* Fix loading indicator not being shown which also caused multiple map zooms during route requests
@karussell
Copy link
Member

I have no good idea yet how to implement a selection on the routeing result list level.

And if we also "uncollapse" the bottom "alternative routes" panel when clicking on it or dragging it? qwant does a similar thing. And once the map is selected again, the panel will collapse again and show only the currently selected route.

The 'Powered by GraphHopper` logo still kind of takes a lot of space and since the url is (or will be) graphhopper.com/maps I am not sure if this is really necessary.

For "advertisement purposes" :) at least one link to graphhopper.com would be important. With this separate panel we can also point customers to GraphHopper Maps on how to properly attribute the GraphHopper Directions API usage. But we can also hide the attribution panel on mobile and move the GraphHopper attribution into the newly added map attribution (i) icon?

btw: there is now a layout bug that the logo is above the auto complete list (for desktop).

# Conflicts:
#	src/map/Mapbox.ts
#	src/sidebar/Sidebar.tsx
#	src/sidebar/search/Search.tsx
Also introduce context menu on long touch
@Janekdererste
Copy link
Collaborator Author

And if we also "uncollapse" the bottom "alternative routes" panel when clicking on it or dragging it? qwant does a similar thing. And once the map is selected again, the panel will collapse again and show only the currently selected route.

That's a very good idea, and that's what e.g. the iOS Maps app does as well. Will open an new issue (#79 ) for this though because I want to merge this PR and I like to split up work into smaller peaces.

The Geocoding issues will be fixed in #54

selecting a different map on mobile nearly always results in selecting kurviger first before I can open it

Create issue #80 for this

@Janekdererste
Copy link
Collaborator Author

fixes #44

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.

None yet

3 participants