Skip to content

Refactor nearby classes mvp #2969

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

Merged
merged 161 commits into from
Oct 15, 2019

Conversation

neslihanturan
Copy link
Collaborator

@neslihanturan neslihanturan commented May 27, 2019

Refactors current nearby implementation to mvp structure

Fixes #2884 and some parts of #1092
Known issues:

  • Does not update the map after Wikidata edits
    Solved
  • Displays error dialog even if no error occurred
    Currently error dialog is not displayed, but the reason of NPE throwable can be detected on another PR.

Other functions are ready to be tested finally:)

…ement in fragment. Call them from presenter by passing locationServiceManager parameter
…ter all views and fragments are ready and attached
app/build.gradle Outdated
@@ -137,7 +138,7 @@ android {
test.resources.srcDirs += 'src/main/resoures'
}

signingConfigs {
signingConfigs {
Copy link
Member

Choose a reason for hiding this comment

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

Please undo unrelated formatting changes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do you know an easy way to see exact number of spaces of original file? AS formats them automatically otherwise

Copy link
Member

Choose a reason for hiding this comment

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

Our project uses Google Code styling. You can download and set the styling in your IDE. After setting it, Android Studio will format it consistently. https://google.github.io/styleguide/javaguide.html

Also, in your IDE you should set to formot only VCS changed files and not all files.

@@ -36,7 +35,6 @@
this.imageUrl = imageUrl;
}

@NotNull
Copy link
Member

Choose a reason for hiding this comment

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

Can you briefly explain why this annotation was removed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The annotation import causes build error.

Copy link
Member

Choose a reason for hiding this comment

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

Can you share the build error that was caused by this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

/apps-android-commons/app/src/main/java/fr/free/nrw/commons/mwapi/UploadResult.java:39: error: cannot find symbol
    @NotNull
     ^
  symbol:   class NotNull
  location: class UploadResult

It seems like this on master too:
Screenshot from 2019-10-05 21-59-10

But somehow it does not cause a build issue on master.

Copy link
Member

Choose a reason for hiding this comment

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

The import is incorrect. You can use

import android.support.annotation.NonNull

// Otherwise go back to contributions fragment
viewPager.setCurrentItem(0);
}
NearbyParentFragment nearbyFragment = (NearbyParentFragment) contributionsActivityPagerAdapter.getItem(1);
Copy link
Member

Choose a reason for hiding this comment

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

@neslihanturan Yes, the presenter should handle user actions but getting an instance of the fragment to get an instance of the presenter is not the right way of doing it.

Maintain a singleton instance of the presenter which can be accessed without depending on fragment lifecycle.

@@ -117,7 +120,7 @@ protected void hookListeners(View view) {
}
}
if (onBookmarkClick == null) {
((NearbyFragment) fragment.getParentFragment()).centerMapToPlace(place);
((NearbyParentFragment) fragment.getParentFragment()).centerMapToPlace(place);
Copy link
Member

Choose a reason for hiding this comment

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

Again, the fragment should not be called from another class to access centerMapToPlace. Ideally there could be a interface or event publishing.


PolygonOptions currentLocationPolygonOptions = new PolygonOptions()
.addAll(circle)
.strokeColor(Color.parseColor("#55000000"))
Copy link
Member

Choose a reason for hiding this comment

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

Define these colors in colors.xml

}


private void initViews() {
Copy link
Member

Choose a reason for hiding this comment

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

Break initViews into smaller methods. As of now all UI initialisation is happening in the same method.

MapboxMapOptions options = new MapboxMapOptions()
.compassGravity(Gravity.BOTTOM | Gravity.LEFT)
.compassMargins(new int[]{12, 0, 0, 24})
//.styleUrl(isDarkTheme ? Style.DARK : Style.OUTDOORS)
Copy link
Member

Choose a reason for hiding this comment

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

is this intentionally commented out?

@Override
public void populatePlaces(fr.free.nrw.commons.location.LatLng curlatLng, fr.free.nrw.commons.location.LatLng searchLatLng) {
boolean checkingAroundCurrentLocation;
if (curlatLng.equals(searchLatLng)) { // Means we are checking around current location
Copy link
Member

Choose a reason for hiding this comment

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

break into smaller methods. :)

@neslihanturan
Copy link
Collaborator Author

Hi @neslihanturan , I managed to test it while walking around today. My marker does not appear to move when I do, similar to the user's complaint above. Does it work for you?

@misaochan can you please link which user above?

@misaochan
Copy link
Member

android:layout_height="wrap_content"

/>
<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this name space needed over here ?

break;
case NEARBY_TAB_POSITION:
Timber.d("Nearby tab selected");
tabLayout.getTabAt(NEARBY_TAB_POSITION).select();
isContributionsFragmentVisible = false;
updateMenuItem();
// Do all permission and GPS related tasks on tab selected, not on create
((NearbyFragment)contributionsActivityPagerAdapter.getItem(1)).onTabSelected(onOrientationChanged);
NearbyParentFragmentPresenter.getInstance().onTabSelected();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shall we use callbacks for this thing, that way code would be cleaner and by defining contracts even novice contributors would know the intended use case

Copy link
Member

Choose a reason for hiding this comment

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

Yes I agree with @ashishkumar468. Let us use callbacks.

app/build.gradle Outdated
@@ -137,7 +138,7 @@ android {
test.resources.srcDirs += 'src/main/resoures'
}

signingConfigs {
signingConfigs {
Copy link
Member

Choose a reason for hiding this comment

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

Our project uses Google Code styling. You can download and set the styling in your IDE. After setting it, Android Studio will format it consistently. https://google.github.io/styleguide/javaguide.html

Also, in your IDE you should set to formot only VCS changed files and not all files.

break;
case NEARBY_TAB_POSITION:
Timber.d("Nearby tab selected");
tabLayout.getTabAt(NEARBY_TAB_POSITION).select();
isContributionsFragmentVisible = false;
updateMenuItem();
// Do all permission and GPS related tasks on tab selected, not on create
((NearbyFragment)contributionsActivityPagerAdapter.getItem(1)).onTabSelected(onOrientationChanged);
NearbyParentFragmentPresenter.getInstance().onTabSelected();
Copy link
Member

Choose a reason for hiding this comment

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

Yes I agree with @ashishkumar468. Let us use callbacks.

// Otherwise go back to contributions fragment
viewPager.setCurrentItem(0);
}
NearbyParentFragment nearbyFragment = (NearbyParentFragment) contributionsActivityPagerAdapter.getItem(1);
Copy link
Member

Choose a reason for hiding this comment

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

@neslihanturan Can you address this comment.

@@ -351,7 +331,7 @@ public boolean onOptionsItemSelected(MenuItem item) {
return true;
case R.id.list_sheet:
if (contributionsActivityPagerAdapter.getItem(1) != null) {
((NearbyFragment)contributionsActivityPagerAdapter.getItem(1)).listOptionMenuItemClicked();
((NearbyParentFragment)contributionsActivityPagerAdapter.getItem(1)).listOptionMenuItemClicked();
Copy link
Member

Choose a reason for hiding this comment

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

As explained above, please use callback here as well.

import fr.free.nrw.commons.nearby.NearbyFragment;
import fr.free.nrw.commons.nearby.NearbyListFragment;
import fr.free.nrw.commons.nearby.NearbyMapFragment;
import fr.free.nrw.commons.nearby.mvp.fragments.NearbyListFragment;
Copy link
Member

Choose a reason for hiding this comment

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

I just noticed this directory structure. Ideally mvp folder isn't needed

Comment on lines 420 to 428
NearbyBaseMarker nearbyBaseMarker = new NearbyBaseMarker();
nearbyBaseMarker.title(place.name);
nearbyBaseMarker.position(
new com.mapbox.mapboxsdk.geometry.LatLng(
place.location.getLatitude(),
place.location.getLongitude()));
nearbyBaseMarker.place(place);
nearbyBaseMarker.icon(IconFactory.getInstance(getContext())
.fromBitmap(icon));
Copy link
Member

Choose a reason for hiding this comment

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

Can be a separate function.

Comment on lines 447 to 453
CameraPosition position = new CameraPosition.Builder()
.target(LocationUtils.commonsLatLngToMapBoxLatLng(
new LatLng(place.location.getLatitude()-cameraShift,
place.getLocation().getLongitude(),
0))) // Sets the new camera position
.zoom(ZOOM_LEVEL) // Same zoom level
.build();
Copy link
Member

Choose a reason for hiding this comment

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

It can be a separate function.

isDarkTheme = applicationKvStore.getBoolean("theme", false);
MapboxMapOptions options = new MapboxMapOptions()
.compassGravity(Gravity.BOTTOM | Gravity.LEFT)
.compassMargins(new int[]{12, 0, 0, 24})
Copy link
Member

Choose a reason for hiding this comment

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

define the array as a constant. name the constant as something meaningful so that anyone can understand what it means. :)

.attributionEnabled(false)
.camera(new CameraPosition.Builder()
.zoom(ZOOM_LEVEL)
.target(new com.mapbox.mapboxsdk.geometry.LatLng(-52.6885, -70.1395))
Copy link
Member

Choose a reason for hiding this comment

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

Which coordinate is this? Extract to a named constant if it is just a default value.


public static NearbyParentFragmentPresenter presenterInstance;

private NearbyParentFragmentPresenter(NearbyParentFragmentContract.NearbyListView nearbyListFragmentView,
Copy link
Member

Choose a reason for hiding this comment

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

IMO this presenter can be a singleton. @ashishkumar468 opinions?

@misaochan
Copy link
Member

misaochan commented Oct 5, 2019

Re: the location updates, the results of my tests:

  • "Location changing slightly" works. The updates are a bit jerky - the dot didn't move with me but rather updated periodically. That's not a big issue though, we can release with that.

  • "Location changed significantly" produces a strange-looking result. It technically works but the new location looks like this:

Screenshot_1570273808

2019-10-05 21:08:28.911 13112-13112/fr.free.nrw.commons D/NearbyParentFragmentPresenter: Presenter updates map and list
2019-10-05 21:08:28.912 13112-13112/fr.free.nrw.commons D/NearbyParentFragmentPresenter: LOCATION_SIGNIFICANTLY_CHANGED
2019-10-05 21:08:28.913 13112-13183/fr.free.nrw.commons D/NearbyController: Loading attractions near lat/lng: (37.409998333333334,-122.08400000000002)
2019-10-05 21:08:28.939 13112-13183/fr.free.nrw.commons D/NearbyPlaces: 2 results at radius: 1.000000
2019-10-05 21:08:28.958 13112-13183/fr.free.nrw.commons D/NearbyPlaces: 4 results at radius: 1.618000
2019-10-05 21:08:29.001 13112-13183/fr.free.nrw.commons D/NearbyPlaces: 22 results at radius: 2.617924
2019-10-05 21:08:29.101 13112-13183/fr.free.nrw.commons D/NearbyPlaces: 57 results at radius: 4.235801
2019-10-05 21:08:29.101 13112-13183/fr.free.nrw.commons D/NearbyController: Sorting places by distance...
2019-10-05 21:08:29.103 13112-13112/fr.free.nrw.commons D/NearbyMapFragment: Updates map markers
2019-10-05 21:08:29.155 13112-13112/fr.free.nrw.commons D/NearbyMapFragment: Adds current location marker
2019-10-05 21:08:29.157 13112-13112/fr.free.nrw.commons D/NearbyMapFragment: Updates map camera to track user position
2019-10-05 21:08:29.158 13112-13112/fr.free.nrw.commons D/NearbyListFragment: Update list fragment
2019-10-05 21:08:51.215 13112-13112/fr.free.nrw.commons D/LocationServiceManager: gps's status changed to 1
2019-10-05 21:09:03.230 13112-13112/fr.free.nrw.commons D/LocationServiceManager: gps's status changed to 1
2019-10-05 21:09:15.227 13112-13112/fr.free.nrw.commons D/LocationServiceManager: gps's status changed to 1
2019-10-05 21:09:39.234 13112-13112/fr.free.nrw.commons D/LocationServiceManager: on location changed
2019-10-05 21:09:39.234 13112-13112/fr.free.nrw.commons D/NearbyParentFragmentPresenter: Location significantly changed
2019-10-05 21:09:39.234 13112-13112/fr.free.nrw.commons D/NearbyParentFragmentPresenter: Presenter updates map and list
2019-10-05 21:09:39.235 13112-13112/fr.free.nrw.commons D/NearbyParentFragmentPresenter: LOCATION_SIGNIFICANTLY_CHANGED
2019-10-05 21:09:39.237 13112-13183/fr.free.nrw.commons D/NearbyController: Loading attractions near lat/lng: (-27.458041666666666,153.05009166666665)
2019-10-05 21:09:39.239 13112-13112/fr.free.nrw.commons D/LocationServiceManager: gps's status changed to 1
2019-10-05 21:09:39.250 13112-13112/fr.free.nrw.commons D/TelemetryService: Locations reported: 1
2019-10-05 21:09:39.250 13112-13441/fr.free.nrw.commons D/NearbyController: Loading attractions near lat/lng: (-27.458041666666666,153.05009166666665)
2019-10-05 21:09:39.262 13112-13123/fr.free.nrw.commons I/zygote: Do partial code cache collection, code=987KB, data=590KB
2019-10-05 21:09:39.266 13112-13123/fr.free.nrw.commons I/zygote: After code cache collection, code=987KB, data=590KB
2019-10-05 21:09:39.266 13112-13123/fr.free.nrw.commons I/zygote: Increasing code cache capacity to 3MB
2019-10-05 21:09:39.972 13112-13183/fr.free.nrw.commons D/NearbyPlaces: 8 results at radius: 1.000000
2019-10-05 21:09:40.478 13112-13441/fr.free.nrw.commons D/NearbyPlaces: 8 results at radius: 1.618000
2019-10-05 21:09:40.478 13112-13441/fr.free.nrw.commons D/NearbyController: Sorting places by distance...
2019-10-05 21:09:40.479 13112-13112/fr.free.nrw.commons D/NearbyNotificationCardView: Update nearby card notification content
2019-10-05 21:09:40.807 13112-13183/fr.free.nrw.commons D/NearbyPlaces: 28 results at radius: 1.618000
2019-10-05 21:09:42.621 13112-13183/fr.free.nrw.commons D/NearbyPlaces: 91 results at radius: 2.617924
2019-10-05 21:09:42.622 13112-13183/fr.free.nrw.commons D/NearbyController: Sorting places by distance...
2019-10-05 21:09:42.624 13112-13112/fr.free.nrw.commons D/NearbyMapFragment: Updates map markers
2019-10-05 21:09:42.694 13112-13112/fr.free.nrw.commons D/NearbyMapFragment: Adds current location marker
2019-10-05 21:09:42.695 13112-13112/fr.free.nrw.commons D/NearbyMapFragment: Updates map camera to track user position
2019-10-05 21:09:42.697 13112-13112/fr.free.nrw.commons D/NearbyListFragment: Update list fragment

@neslihanturan
Copy link
Collaborator Author

@misaochan how do you reproduce weird looking significant location update?

@misaochan
Copy link
Member

Take a car ride or spoof your GPS.

@misaochan misaochan mentioned this pull request Oct 5, 2019
12 tasks
@neslihanturan
Copy link
Collaborator Author

Sorry @misaochan I can not reproduce it. Are you sure there is not a specific use case to create this issue? My significant changes works just fine.

@misaochan
Copy link
Member

Sorry @misaochan I can not reproduce it. Are you sure there is not a specific use case to create this issue? My significant changes works just fine.

Hmm, I could not reproduce it today either. Once the code review part is sorted, I'm happy to merge into master and see if anyone else reports it. :)

@misaochan
Copy link
Member

Thanks @neslihanturan !

@misaochan misaochan merged commit 44cdd2f into commons-app:master Oct 15, 2019
maskaravivek pushed a commit that referenced this pull request Oct 15, 2019
* Create parent contract

* Create map child contract and fill methods

* Add javadocs and specific interfaces for list

* Move general method to parent and add javadocs for parent

* Add explanation for keeping an emty View interface under NearbyListContract

* Move constracts under contract package

* Create presenters for map and list and implement user actions accordingly

* Add javadocs

* Add presenter, contract and fragment for parent Fragment of both NearyListFragment and NearbyMapFragment

* Implement missing methods

* Fix typo

* Add main views on fragment

* İmplement child fragment logic and their retain

* Relate parent presenter with parent fragment

* Add all location permission related methods to view contract and implement in fragment. Call them from presenter by passing locationServiceManager parameter

* Define refreshView method as updateMapAndList which is a better naming. Define it at presenter part.

* Define a presenter variable in fragment and call updateMapAndList method from there, if permissions are okay

* Add lock neabry method to unlisten nearby operations during updates

* Add network connection established check on view side, check it from presenter

* try to simplify previous method during refactor

* Add missing methods for NearbyMap

* Connect child fragment and prent fragment with presenter

* Change nearby design, first create views then register listeners

* AddnetworkBroadcatsReceiver on view side, call it from presenter

* Add comments

* Change the old NearbyFragment by our new NearbyParentFragment for first tests

* Prevent crash caused child fragment is actually null by checking if it is attached or not.

* Makes sure that initialize nearby operations method is called just after all views and fragments are ready and attached

* Make sure updateMAoAndList method is called when everything ready

* Rename a method with prepoer name

* Call update map and add required markers

* Find out zoom level problem

* Implement add nearby markers and add current marker methods

* remove unneeded codes copied from previous implementation

* Revert "remove unneeded codes copied from previous implementation"

This reverts commit 4253965.

* add some commits and clear code

* Remove location listener implementation from view, handle them in presenter instead. And add timber debug notes and required method calls

* Style ,issues

* Refactor a variable name to camel case and bind search this area view

* Search this area button action is added

* Mostly implement search this area methods, not tested yet even once

* Make sure everything is called in required order and seach this area method basically works

* Rename methods accordingly and add Javadocs

* Add current location marker and remove circle around it

* Remove unused methods

* Add current location marker with object animator and remove previous marker is exists

* include clear map into add markers method and reorder methods

* Make search this area button appear at correct time

* Try to load un search this area is called

* Clear logs

* Add changes for permission made by Vivek to newly added fragments along with our nearby classes

* Add a view to nearby map ragment and insert map view as an item inside it

* Add logs to uınderstand why on meap ready callback is never called

* Add list item clicked and bottom sheet for list of nearby items is expanded

* Make nearby map ready callback came

* Add required methods to be called after map view is ready

* State: Map ready call is not called but permissions and methods are in correct order

* Remove unused logs

* Try to use SupportMapFragment instead, still no success...

* use SupportMapFragment instead

* Remove unused Near

* Upgrade mapbox sdk versions

* Remove Style import from fragment/NearbyMapFragment

* Remove Style import from **fragments/NearbyParentFragment

* Remove nearby/NearbyMapFragment

* Remove unused/old NearbyFragment and NearbyMapFragment

* Remove import of already removed class

* Make sure you removed everything related with mapbox implementation

* Update mapbox map

* Remove unused classes, do not forget centerMapToPlace doesn't work on this branch

* Remove Style class it required updated version of mapbox map

* Add base codes for new activity

* Prove that our mapbox sdk let us implement the map directly inside the activity

* Add base codes for testing mapbox activity in a single fragment inside activity

* Add codes from mapbox demo repository to test support fragment, map works in a single layer fragment

* Add base codes for test layered fragment activity

* Add Support fragment inside a fragment and proves that layered fragmentw with map works

* Test view pager and tab layout with support map fragment to see it works, it works!

* Move Contributions Fragment related codes to test activity

* Move nearby card methods

* Inject location manager and implement NerabyParentFragmentContract.View on test fragment

* Coppy the content of SupportMapFragment from mapbox repository, use this code to modify later. This method war suggested by mapbox team instead of extending the class

* Implement NearbyMapContract.View on our new SupportMapFragment

* Start to mplement logic of checking permissions

* Fix small dagger issue to inject location manager properly

* Request permission for nearby places if fragment is loaded and tab is selected

* Initialize map operations if map ready and tab is selected

* Markers loads at correct time

* Style the map according to new version of mapbox map

* Add some map elements like FABs and give their actions

* Implement map marker click actions

* Implement nearby Fabs logic with a small issue

* fix FABs are not closing problem

* Unkown problem occurs at map load when I try to use MainActivity again.

* Revert "Unkown problem occurs at map load when I try to use MainActivity again."

This reverts commit 3dc0844.

* Search this area buttons are added but button function does not work and button visibility is problematic

* Fix issue with MainActivity with the help of Ashish

* Fix search this area button visibility issue

* Fix the issues with updating search nearby markers and camera position together

* Fix progress bar visibility issue

* Prevent loding map each time tab selected

* Take toolbar back

* Implement back button with presenter

* Add click actoion to bottom sheet details

* Add nearby list into bottom sheeet

* Make reuse existing fragments if there is any

* Code cleanup

* Cleanup

* Code cleanup

* Add lifecyle codes to prevent leaks

* Cleanup

* Code cleanup

* cleanup

* Make list item clicked and map focus to same place

* Add bookmark from list fragment

* Make nearby card click action work

* Revert "Fix conflicts"

This reverts commit f345174, reversing
changes made to c5d4d55.

* Code cleanup

* Cleanup

* Make recenter button work when list sheet is expanded

* Cleanup

* Code cleanups

* NPE issue is not detected for now, can be solved on seperate PR

* Update map after Wikidata edit

* Cherry picked previously reverted merge, hoping this will fix enourmus amounts of diff files

* Previous merge bringed an file which should be deleted. Delete it.

* Revert irrelevant changes on build.gradle

* Revert irrelevant changes

* Jetbrains annotation is not included to build gradle, this issue caused build issues on my branch so I removed it to be able to build

* Rename ListView

* Use a singleton to access the presenter, instead of passing it to a variable inside fragment

* Fix code style issues

* Move hardcoded colors to colors.xml file

* Make larger methods smaller

* Make current location marker follow

* Do not track users position if user is searching around

* Revert irrelevant shanges at build gradle

* Remove unneeded variable

* Remove mvp directory, add sub directories directly under nearby directory instead

* Remove unneeded public definiton

* Remove unneeded namespace

* Add public defiiton, it is needed to reach the constructor.
misaochan pushed a commit that referenced this pull request Oct 16, 2019
* Create parent contract

* Create map child contract and fill methods

* Add javadocs and specific interfaces for list

* Move general method to parent and add javadocs for parent

* Add explanation for keeping an emty View interface under NearbyListContract

* Move constracts under contract package

* Create presenters for map and list and implement user actions accordingly

* Add javadocs

* Add presenter, contract and fragment for parent Fragment of both NearyListFragment and NearbyMapFragment

* Implement missing methods

* Fix typo

* Add main views on fragment

* İmplement child fragment logic and their retain

* Relate parent presenter with parent fragment

* Add all location permission related methods to view contract and implement in fragment. Call them from presenter by passing locationServiceManager parameter

* Define refreshView method as updateMapAndList which is a better naming. Define it at presenter part.

* Define a presenter variable in fragment and call updateMapAndList method from there, if permissions are okay

* Add lock neabry method to unlisten nearby operations during updates

* Add network connection established check on view side, check it from presenter

* try to simplify previous method during refactor

* Add missing methods for NearbyMap

* Connect child fragment and prent fragment with presenter

* Change nearby design, first create views then register listeners

* AddnetworkBroadcatsReceiver on view side, call it from presenter

* Add comments

* Change the old NearbyFragment by our new NearbyParentFragment for first tests

* Prevent crash caused child fragment is actually null by checking if it is attached or not.

* Makes sure that initialize nearby operations method is called just after all views and fragments are ready and attached

* Make sure updateMAoAndList method is called when everything ready

* Rename a method with prepoer name

* Call update map and add required markers

* Find out zoom level problem

* Implement add nearby markers and add current marker methods

* remove unneeded codes copied from previous implementation

* Revert "remove unneeded codes copied from previous implementation"

This reverts commit 4253965.

* add some commits and clear code

* Remove location listener implementation from view, handle them in presenter instead. And add timber debug notes and required method calls

* Style ,issues

* Refactor a variable name to camel case and bind search this area view

* Search this area button action is added

* Mostly implement search this area methods, not tested yet even once

* Make sure everything is called in required order and seach this area method basically works

* Rename methods accordingly and add Javadocs

* Add current location marker and remove circle around it

* Remove unused methods

* Add current location marker with object animator and remove previous marker is exists

* include clear map into add markers method and reorder methods

* Make search this area button appear at correct time

* Try to load un search this area is called

* Clear logs

* Add changes for permission made by Vivek to newly added fragments along with our nearby classes

* Add a view to nearby map ragment and insert map view as an item inside it

* Add logs to uınderstand why on meap ready callback is never called

* Add list item clicked and bottom sheet for list of nearby items is expanded

* Make nearby map ready callback came

* Add required methods to be called after map view is ready

* State: Map ready call is not called but permissions and methods are in correct order

* Remove unused logs

* Try to use SupportMapFragment instead, still no success...

* use SupportMapFragment instead

* Remove unused Near

* Upgrade mapbox sdk versions

* Remove Style import from fragment/NearbyMapFragment

* Remove Style import from **fragments/NearbyParentFragment

* Remove nearby/NearbyMapFragment

* Remove unused/old NearbyFragment and NearbyMapFragment

* Remove import of already removed class

* Make sure you removed everything related with mapbox implementation

* Update mapbox map

* Remove unused classes, do not forget centerMapToPlace doesn't work on this branch

* Remove Style class it required updated version of mapbox map

* Add base codes for new activity

* Prove that our mapbox sdk let us implement the map directly inside the activity

* Add base codes for testing mapbox activity in a single fragment inside activity

* Add codes from mapbox demo repository to test support fragment, map works in a single layer fragment

* Add base codes for test layered fragment activity

* Add Support fragment inside a fragment and proves that layered fragmentw with map works

* Test view pager and tab layout with support map fragment to see it works, it works!

* Move Contributions Fragment related codes to test activity

* Move nearby card methods

* Inject location manager and implement NerabyParentFragmentContract.View on test fragment

* Coppy the content of SupportMapFragment from mapbox repository, use this code to modify later. This method war suggested by mapbox team instead of extending the class

* Implement NearbyMapContract.View on our new SupportMapFragment

* Start to mplement logic of checking permissions

* Fix small dagger issue to inject location manager properly

* Request permission for nearby places if fragment is loaded and tab is selected

* Initialize map operations if map ready and tab is selected

* Markers loads at correct time

* Style the map according to new version of mapbox map

* Add some map elements like FABs and give their actions

* Implement map marker click actions

* Implement nearby Fabs logic with a small issue

* fix FABs are not closing problem

* Unkown problem occurs at map load when I try to use MainActivity again.

* Revert "Unkown problem occurs at map load when I try to use MainActivity again."

This reverts commit 3dc0844.

* Search this area buttons are added but button function does not work and button visibility is problematic

* Fix issue with MainActivity with the help of Ashish

* Fix search this area button visibility issue

* Fix the issues with updating search nearby markers and camera position together

* Fix progress bar visibility issue

* Prevent loding map each time tab selected

* Take toolbar back

* Implement back button with presenter

* Add click actoion to bottom sheet details

* Add nearby list into bottom sheeet

* Make reuse existing fragments if there is any

* Code cleanup

* Cleanup

* Code cleanup

* Add lifecyle codes to prevent leaks

* Cleanup

* Code cleanup

* cleanup

* Make list item clicked and map focus to same place

* Add bookmark from list fragment

* Make nearby card click action work

* Revert "Fix conflicts"

This reverts commit f345174, reversing
changes made to c5d4d55.

* Code cleanup

* Cleanup

* Make recenter button work when list sheet is expanded

* Cleanup

* Code cleanups

* NPE issue is not detected for now, can be solved on seperate PR

* Update map after Wikidata edit

* Cherry picked previously reverted merge, hoping this will fix enourmus amounts of diff files

* Previous merge bringed an file which should be deleted. Delete it.

* Revert irrelevant changes on build.gradle

* Revert irrelevant changes

* Jetbrains annotation is not included to build gradle, this issue caused build issues on my branch so I removed it to be able to build

* Rename ListView

* Use a singleton to access the presenter, instead of passing it to a variable inside fragment

* Fix code style issues

* Move hardcoded colors to colors.xml file

* Make larger methods smaller

* Make current location marker follow

* Do not track users position if user is searching around

* Revert irrelevant shanges at build gradle

* Remove unneeded variable

* Remove mvp directory, add sub directories directly under nearby directory instead

* Remove unneeded public definiton

* Remove unneeded namespace

* Add public defiiton, it is needed to reach the constructor.
ashishkumar468 pushed a commit that referenced this pull request Nov 6, 2019
* Migrated logEvents to retrofit (#3087)

* Switched wikimedia-android-data-client for new features

* Added UserCLient and UserInterface

* Migrated ContributionsSyncAdapter to UserClient
Fixed sync related bugs

* Removed unused code

* Removed unused code

* Updated wikimedia-android-data-client to new version

* Update library for data client (#3131)

* Backend overhaul fetch media by filename (#3081)

* Added class MwParseResponse and MwParseResult for receiving parse output

* Migrated fetchMediaByFilename to retrofit

* Removed unused code

* Added tests

* Migrated isUserBlockedFromCommons to retrofit (#3085)

* Switched wikimedia-android-data-client for new features

* Added UserCLient and UserInterface

* Migrated isUserBlockedFromCommons to retrofit

* Added tests and removed dead code

* Implemented ban checking functionality in UploadActivity

* Removed unused class AuthenticatedActivity

* Fixed tests

* Fixed NullPointerException when a user accessed image details without logging in.

* * Login the login token way, handle LoginResult minutely, add account based on LoginResult (#3151)

* Added progress updater in UploadService to show upload progress in no… (#3156)

* added progress updater in UploadService to show upload progress in notification

* formatting changes

* [Dependency: Quadtree] Remove unused code from cache controller (#3163)

* Basic logging with redacted sensitive headers (#3159)

* As per #3026, removed the obsolete classes in package mwapi (#3150)

* fixed compile time error (#3165)

* [Dependency fluent]: Remove unused dependency fluent (#3164)

* Donot init quiz checker in onResume (#3167)

* clear image cache on logout (#3168)

* Fixed default locale and upload locales in descrptions (#3166)

* Fix 2FA login (#3170)

* Fix build (#3172)

* Localisation updates from https://translatewiki.net.

* Closes #3094 (#3095)

* BugFix in SpinnerDescriptionsAdapter and SpinnerLanguagesAdapter (use the langguage code provided by the spinner, donot set the language to the one returned by the locale)

* Update changelog.md

* Versioning for v2.11.0

* Localisation updates from https://translatewiki.net.

* Localisation updates from https://translatewiki.net.

* Localisation updates from https://translatewiki.net.

* Center map on location clicked in nearby list and notification card(#2060) (#2366)

* center map on location clicked in nearby (#2060)

* improved animation

* Center map on location clicked in nearby list

* removed unnecessary methods

* center map on location clicked in nearby notification card

* some minor changes

* travis build error

* resolved errors

* Tidy up PR

* removed swipe to delete (#2589)

* Localisation updates from https://translatewiki.net.

* Localisation updates from https://translatewiki.net.

* Localisation updates from https://translatewiki.net.

* Localisation updates from https://translatewiki.net.

* Localisation updates from https://translatewiki.net.

* Localisation updates from https://translatewiki.net.

* edited CREDITS file (#3145)

* Unused dependencies are removed (#3141)

Part of #3128

* Localisation updates from https://translatewiki.net.

* Localisation updates from https://translatewiki.net.

* Localisation updates from https://translatewiki.net.

* Working with #3129 issue (#3146)

* Replace Hard-Coded strings with those from strings.xml.

* Localisation updates from https://translatewiki.net.

* Localisation updates from https://translatewiki.net.

* Localisation updates from https://translatewiki.net.

* Localisation updates from https://translatewiki.net.

* delete res/values-en-gb (#3153)

* Localisation updates from https://translatewiki.net.

* Localisation updates from https://translatewiki.net.

* First commit (#3093)

Fixes info icon color is Review

* Solution #3126 (#3155)

Fixed wrong upload dates after image upload

* Bugfix/fix upload presenter tests (#3158)

* Revert "Merge branch 'backend-overhaul' into master"

This reverts commit 0090f24, reversing
changes made to 9bccbfe.

* * Fixed upload presenter tests
* Deleted app/src/main/res/values-en-gb/error.xml

* Localisation updates from https://translatewiki.net.

* Remove dependency on Exif parsing library. (#2947)

* Remove dependency on Exif parsing library.

* Fix test.

* Localisation updates from https://translatewiki.net.

* Localisation updates from https://translatewiki.net.

* Refactor nearby classes mvp (#2969)

* Create parent contract

* Create map child contract and fill methods

* Add javadocs and specific interfaces for list

* Move general method to parent and add javadocs for parent

* Add explanation for keeping an emty View interface under NearbyListContract

* Move constracts under contract package

* Create presenters for map and list and implement user actions accordingly

* Add javadocs

* Add presenter, contract and fragment for parent Fragment of both NearyListFragment and NearbyMapFragment

* Implement missing methods

* Fix typo

* Add main views on fragment

* İmplement child fragment logic and their retain

* Relate parent presenter with parent fragment

* Add all location permission related methods to view contract and implement in fragment. Call them from presenter by passing locationServiceManager parameter

* Define refreshView method as updateMapAndList which is a better naming. Define it at presenter part.

* Define a presenter variable in fragment and call updateMapAndList method from there, if permissions are okay

* Add lock neabry method to unlisten nearby operations during updates

* Add network connection established check on view side, check it from presenter

* try to simplify previous method during refactor

* Add missing methods for NearbyMap

* Connect child fragment and prent fragment with presenter

* Change nearby design, first create views then register listeners

* AddnetworkBroadcatsReceiver on view side, call it from presenter

* Add comments

* Change the old NearbyFragment by our new NearbyParentFragment for first tests

* Prevent crash caused child fragment is actually null by checking if it is attached or not.

* Makes sure that initialize nearby operations method is called just after all views and fragments are ready and attached

* Make sure updateMAoAndList method is called when everything ready

* Rename a method with prepoer name

* Call update map and add required markers

* Find out zoom level problem

* Implement add nearby markers and add current marker methods

* remove unneeded codes copied from previous implementation

* Revert "remove unneeded codes copied from previous implementation"

This reverts commit 4253965.

* add some commits and clear code

* Remove location listener implementation from view, handle them in presenter instead. And add timber debug notes and required method calls

* Style ,issues

* Refactor a variable name to camel case and bind search this area view

* Search this area button action is added

* Mostly implement search this area methods, not tested yet even once

* Make sure everything is called in required order and seach this area method basically works

* Rename methods accordingly and add Javadocs

* Add current location marker and remove circle around it

* Remove unused methods

* Add current location marker with object animator and remove previous marker is exists

* include clear map into add markers method and reorder methods

* Make search this area button appear at correct time

* Try to load un search this area is called

* Clear logs

* Add changes for permission made by Vivek to newly added fragments along with our nearby classes

* Add a view to nearby map ragment and insert map view as an item inside it

* Add logs to uınderstand why on meap ready callback is never called

* Add list item clicked and bottom sheet for list of nearby items is expanded

* Make nearby map ready callback came

* Add required methods to be called after map view is ready

* State: Map ready call is not called but permissions and methods are in correct order

* Remove unused logs

* Try to use SupportMapFragment instead, still no success...

* use SupportMapFragment instead

* Remove unused Near

* Upgrade mapbox sdk versions

* Remove Style import from fragment/NearbyMapFragment

* Remove Style import from **fragments/NearbyParentFragment

* Remove nearby/NearbyMapFragment

* Remove unused/old NearbyFragment and NearbyMapFragment

* Remove import of already removed class

* Make sure you removed everything related with mapbox implementation

* Update mapbox map

* Remove unused classes, do not forget centerMapToPlace doesn't work on this branch

* Remove Style class it required updated version of mapbox map

* Add base codes for new activity

* Prove that our mapbox sdk let us implement the map directly inside the activity

* Add base codes for testing mapbox activity in a single fragment inside activity

* Add codes from mapbox demo repository to test support fragment, map works in a single layer fragment

* Add base codes for test layered fragment activity

* Add Support fragment inside a fragment and proves that layered fragmentw with map works

* Test view pager and tab layout with support map fragment to see it works, it works!

* Move Contributions Fragment related codes to test activity

* Move nearby card methods

* Inject location manager and implement NerabyParentFragmentContract.View on test fragment

* Coppy the content of SupportMapFragment from mapbox repository, use this code to modify later. This method war suggested by mapbox team instead of extending the class

* Implement NearbyMapContract.View on our new SupportMapFragment

* Start to mplement logic of checking permissions

* Fix small dagger issue to inject location manager properly

* Request permission for nearby places if fragment is loaded and tab is selected

* Initialize map operations if map ready and tab is selected

* Markers loads at correct time

* Style the map according to new version of mapbox map

* Add some map elements like FABs and give their actions

* Implement map marker click actions

* Implement nearby Fabs logic with a small issue

* fix FABs are not closing problem

* Unkown problem occurs at map load when I try to use MainActivity again.

* Revert "Unkown problem occurs at map load when I try to use MainActivity again."

This reverts commit 3dc0844.

* Search this area buttons are added but button function does not work and button visibility is problematic

* Fix issue with MainActivity with the help of Ashish

* Fix search this area button visibility issue

* Fix the issues with updating search nearby markers and camera position together

* Fix progress bar visibility issue

* Prevent loding map each time tab selected

* Take toolbar back

* Implement back button with presenter

* Add click actoion to bottom sheet details

* Add nearby list into bottom sheeet

* Make reuse existing fragments if there is any

* Code cleanup

* Cleanup

* Code cleanup

* Add lifecyle codes to prevent leaks

* Cleanup

* Code cleanup

* cleanup

* Make list item clicked and map focus to same place

* Add bookmark from list fragment

* Make nearby card click action work

* Revert "Fix conflicts"

This reverts commit f345174, reversing
changes made to c5d4d55.

* Code cleanup

* Cleanup

* Make recenter button work when list sheet is expanded

* Cleanup

* Code cleanups

* NPE issue is not detected for now, can be solved on seperate PR

* Update map after Wikidata edit

* Cherry picked previously reverted merge, hoping this will fix enourmus amounts of diff files

* Previous merge bringed an file which should be deleted. Delete it.

* Revert irrelevant changes on build.gradle

* Revert irrelevant changes

* Jetbrains annotation is not included to build gradle, this issue caused build issues on my branch so I removed it to be able to build

* Rename ListView

* Use a singleton to access the presenter, instead of passing it to a variable inside fragment

* Fix code style issues

* Move hardcoded colors to colors.xml file

* Make larger methods smaller

* Make current location marker follow

* Do not track users position if user is searching around

* Revert irrelevant shanges at build gradle

* Remove unneeded variable

* Remove mvp directory, add sub directories directly under nearby directory instead

* Remove unneeded public definiton

* Remove unneeded namespace

* Add public defiiton, it is needed to reach the constructor.

* On Logout, fetch the CSRF token and then make the post logout call (#3182)

* Api call for logout

* call clear cached onCompleteSessionLogout

* Correction is passed file name to checkPageExistsUsingTitle, function expects file name prefixed with File: (#3194)

* Merge master (#3196)

* Localisation updates from https://translatewiki.net.

* Localisation updates from https://translatewiki.net.

* Localisation updates from https://translatewiki.net.

* Mapped values-en-gb to values-en-rGB (#3161)

Fixes "Error: Invalid resource directory name" bug.

* Localisation updates from https://translatewiki.net.

* Removes the "Other" deletion option mentioned in #3174 (#3183)

* Localisation updates from https://translatewiki.net.

* Renamed resource file to prevent build from failing (#3189)

* Localisation updates from https://translatewiki.net.

* Removed some jargon/slang from strings.xml (#3162)

* Localisation updates from https://translatewiki.net.

* solve issue Avoid 'form movements' in the login screen #1107 (#2936)

* Disabled review buttons while an image is being loaded (#3185)

* Disabled review buttons while an image is being loaded

* Added javadocs for the new methods

* Fix build
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.

Simplify nearby classes, improve code quality
5 participants