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

Feature: open street maps for POIs #137

Merged
merged 23 commits into from
Dec 11, 2020
Merged

Conversation

digorath
Copy link
Contributor

@digorath digorath commented Dec 2, 2020

Changes proposed in this PR:

Added a web view map for points of interest.

The map is shown on the details screen for a specific POI.

Furthermore for a selected category of POIs there now is the option to show it as a map instead. If selected you see the map with all the locations of the POIs. If you tap on one pin, the details are shown below.

In the process of adding it to the IndexScreen (category screen) the screen was refactored.

SVA-86


  1. selected a category -> default view is list view
  2. switched to map view (with no POI selected)
  3. selected a POI
  4. selected a POI from list view
Android iOS
1 Screenshot 2020-12-02 at 15 57 51 Screenshot 2020-12-02 at 16 01 08
2 Screenshot 2020-12-02 at 15 58 12 Screenshot 2020-12-02 at 16 01 18
3 Screenshot 2020-12-02 at 15 58 23 Screenshot 2020-12-02 at 16 01 25
4 Screenshot 2020-12-02 at 15 58 47 Screenshot 2020-12-02 at 16 01 35

@digorath digorath added the enhancement New feature or request label Dec 2, 2020
@digorath digorath self-assigned this Dec 2, 2020
@ghost
Copy link

ghost commented Dec 3, 2020

DeepCode's analysis on #395dec found:

  • ℹ️ 1 minor issue. 👇

Top issues

Description Example fixes
Type declaration of 'any' loses type-safety. Consider replacing it with a more precise type. Occurrences: 🔧 Example fixes

👉 View analysis in DeepCode’s Dashboard | Configure the bot

src/screens/HomeScreen.js Outdated Show resolved Hide resolved
src/components/screens/PointOfInterest.js Show resolved Hide resolved
src/components/DropDownHeader.js Outdated Show resolved Hide resolved
src/components/LoadingContainer.js Show resolved Hide resolved
src/components/MapSwitchHeader.tsx Outdated Show resolved Hide resolved
src/components/map/WebViewMap.tsx Outdated Show resolved Hide resolved
src/components/map/LocationOverview.tsx Outdated Show resolved Hide resolved
src/components/screens/PointOfInterest.js Outdated Show resolved Hide resolved
src/components/map/LocationOverview.tsx Outdated Show resolved Hide resolved
src/components/map/LocationOverview.tsx Outdated Show resolved Hide resolved
Julian Kwast and others added 17 commits December 4, 2020 11:37
- added osm via react-native-webview-leaflet, own fork to align webview versions

SVA-86
- added new screen for testing
- added new component that reacts to map marker clicks in a view below
- added map to point of interest details screen
- overview screen is now fed with data via query
- tapping on a pin will show the point of interest detail card below

SVA-86
- added extra query for getting details of a selected POI

SVA-86
- reordered the setting of constants
- wrapped functions in useCallbacks where necessary
- renamed ListHeader to DropdownHeader
  - we will have a MapSwitchHeader soon

SVA-86
- moved the map further down on details view
- refactored import to get rid of cycle issues

SVA-86
- changed import to get rid of cycles
- changed logic to only show
  - either the activity indicator or the details
  - never both

SVA-86
- added MapSwitchHeader to IndexScreen for POIs
- added possibility to switch between LocationOverview and list for POI
- removed the no longer needed LocationOverviewScreen
- added category filter for LocationOverview

SVA-86
- changed prop type to be MapMarker[] instead of Array<MapMarker>

SVA-86
- used auto formatting and saved the files

SVA-86
- created `ListSwitchItem` file to have a reusable components
  for list switches
- updated `MapSwitchHeader` to use the new components
- removed vertical margin from `LocationOverview` as it resulted
  in weird looking whitespaces on top and at bottom of the map
- adding color for activity indicator
  - and added loading container wrapper to add padding,
    otherwise it would be stacked directly underneath the map

SVA-86
- changed the query for geolocations
- changed components accordingly

SVA-86
- changed the order of arguments to align with prop validation of POI

SVA-86
- fixed casing of DropdownHeader.js and its component

SVA-86
- fixed: JSX element does not have any construct or call signatures.

SVA-86
@digorath digorath force-pushed the feature/SVA-86-open-street-maps branch from fc7c712 to a016f2b Compare December 4, 2020 10:38
- changed WrapperWithOrientation to use the context internally
- added WrapperWithOrientation to LocationOverview
- changed styling of WebViewMap to a default aspect ratio of 16:9
- set default width of WebViewMap to 100%

SVA-86
@digorath digorath requested a review from donni106 December 7, 2020 09:15
Copy link
Member

@donni106 donni106 left a comment

Choose a reason for hiding this comment

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

please check the layout change on orientation change. in my local test this does not behave the way it should.

Julian Kwast added 4 commits December 9, 2020 16:03
- added proper exports/imports through index files
- removed spreading of location with default icon
  - icon is already mandatory in type, so no need to have a fallback

SVA-86
- map size will now be fixed, independent of orientation
- this needs to be done due to a bug in react-native 0.63
(facebook/react-native#29451)

SVA-86
- changed background color to lightest text color

SVA-86
- fixed import grouping in WebViewMap.tsx

SVA-86
@digorath digorath requested a review from donni106 December 9, 2020 15:35
@ghost
Copy link

ghost commented Dec 9, 2020

DeepCode's analysis on #11f10e found:

  • ℹ️ 1 minor issue. 👇

Top issues

Description Example fixes
Type declaration of 'any' loses type-safety. Consider replacing it with a more precise type. Occurrences: 🔧 Example fixes

👉 View analysis in DeepCode’s Dashboard | Configure the bot

@codeclimate
Copy link

codeclimate bot commented Dec 9, 2020

Code Climate has analyzed commit 11f10e4 and detected 0 issues on this pull request.

View more on Code Climate.

@donni106 donni106 added this to the 1.5.0 milestone Dec 9, 2020
@digorath digorath merged commit 76de579 into master Dec 11, 2020
@digorath digorath deleted the feature/SVA-86-open-street-maps branch December 11, 2020 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants