Skip to content

ExampleActivity refactor for simplicity/stability#1884

Merged
devotaaabel merged 5 commits into
masterfrom
devota-update-example-activity
Apr 24, 2019
Merged

ExampleActivity refactor for simplicity/stability#1884
devotaaabel merged 5 commits into
masterfrom
devota-update-example-activity

Conversation

@devotaaabel
Copy link
Copy Markdown
Contributor

@devotaaabel devotaaabel commented Apr 12, 2019

Description

This addresses multiple issues with the ExampleActivity:

  • It simplifies the possible states while adding SEARCH state. Previously, there were SHOW_LOCATION, FIND_ROUTE, ROUTE_FOUND and NAVIGATE states. Now, there's SHOW_LOCATION (just showing users location on the map), SHOW_ROUTE (showing a route either from a destination long clicked on the map, or found via search), SEARCH (search view is open), and NAVIGATE. The directions fab is removed because the intermediary step is no longer needed.

  • Fixes some backstack issues, including pulling in the fix from here and also addressing some of the issues listed in Multiple issues with example activity #1874

  • Retrieved profile and simulate route values from shared preferences as set in settings activity (still have to add onActivityResult)

  • Also added some extra visibility for the settings fab

  • I have added any issue links

  • I have added all related labels (bug, feature, new API(s), SEMVER, etc.)

  • I have added the appropriate milestone and project boards

Goal

The goal is to make this activity simpler, more useful for testing, and less buggy, while also allowing the ability to try different settings.

Implementation

Disclaimer: I'm still having some crazy issues with the camera. These issues exist on master so I don't believe they have anything to do with my changes.

Screenshots or Gifs

Let me know if there's any screenshots/gifs that would help this PR.

Testing

Tested the different settings, profiles/simulate route, and tested that the profiles were translating into the urls. UI tested the example flow, including the search, to try to catch edge cases.

  • I have tested locally (including SNAPSHOT upstream dependencies if needed)
  • I have tested via a test drive, or a simulation/mock location app
  • New and existing unit tests pass locally with my changes

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code

@devotaaabel devotaaabel added refactor feature New feature request. labels Apr 12, 2019
@devotaaabel devotaaabel added this to the v0.35.0 milestone Apr 12, 2019
@devotaaabel devotaaabel self-assigned this Apr 12, 2019
@codecov-io
Copy link
Copy Markdown

codecov-io commented Apr 12, 2019

Codecov Report

Merging #1884 into master will increase coverage by 0.02%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #1884      +/-   ##
============================================
+ Coverage     35.04%   35.07%   +0.02%     
  Complexity     1060     1060              
============================================
  Files           261      261              
  Lines          8848     8839       -9     
  Branches        667      667              
============================================
- Hits           3101     3100       -1     
+ Misses         5475     5467       -8     
  Partials        272      272

@Guardiola31337 Guardiola31337 modified the milestones: v0.35.0, v0.36.0 Apr 12, 2019
@devotaaabel devotaaabel force-pushed the devota-update-example-activity branch from b04c977 to b2862d2 Compare April 15, 2019 13:20
@devotaaabel devotaaabel marked this pull request as ready for review April 15, 2019 15:22
@devotaaabel devotaaabel requested a review from danesfeder April 15, 2019 15:22
@devotaaabel devotaaabel force-pushed the devota-update-example-activity branch 3 times, most recently from 29cf72a to 5208638 Compare April 15, 2019 17:30
Copy link
Copy Markdown
Contributor

@danesfeder danesfeder left a comment

Choose a reason for hiding this comment

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

@devotaaabel first pass here looks good. Will check back when the animation work is good to go as well. Thanks for running with this

@Guardiola31337 Guardiola31337 modified the milestones: v0.36.0, v0.37.0 Apr 17, 2019
@devotaaabel devotaaabel force-pushed the devota-update-example-activity branch from e18803e to 5a23931 Compare April 18, 2019 13:49
@devotaaabel devotaaabel force-pushed the devota-update-example-activity branch from 5a23931 to e21cadd Compare April 18, 2019 17:51
@devotaaabel
Copy link
Copy Markdown
Contributor Author

@mapbox/navigation-android I removed the intro animations from the scope of this PR and and working on that separately. This is ready for final review

Copy link
Copy Markdown
Contributor

@danesfeder danesfeder left a comment

Choose a reason for hiding this comment

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

@devotaaabel just gave this another pass - everything is looking good, just one minor regression where the destination icon is no longer being drawn:

ezgif com-video-to-gif

Once we re-add that, we should be good to go here. Going to go ahead and approve regardless. Thanks!

@devotaaabel devotaaabel merged commit a30d970 into master Apr 24, 2019
@devotaaabel devotaaabel deleted the devota-update-example-activity branch April 24, 2019 16:00
@danesfeder danesfeder mentioned this pull request May 1, 2019
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature request.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants