From 601904f4c61e9fb38289640255aeac49e200c0aa Mon Sep 17 00:00:00 2001 From: Dmitry Donskoy Date: Tue, 6 Aug 2019 17:55:35 +0300 Subject: [PATCH] [android] Fixed review notes https://github.com/mapsme/omim/pull/11340#discussion_r310511103 https://github.com/mapsme/omim/pull/11340#discussion_r310511665 https://github.com/mapsme/omim/pull/11340#discussion_r310564802 https://github.com/mapsme/omim/pull/11340#discussion_r310566828 https://github.com/mapsme/omim/pull/11340#discussion_r310568571 https://github.com/mapsme/omim/pull/11340#discussion_r310574366 --- .../maps/discovery/DiscoveryFragment.java | 16 +++++++--------- .../mapswithme/maps/gallery/impl/Factory.java | 6 ++---- .../maps/settings/SettingsPrefsFragment.java | 5 ++++- .../maps/widget/placepage/PlacePageView.java | 12 ++++++------ .../src/com/mapswithme/util/NetworkPolicy.java | 15 ++++++--------- 5 files changed, 25 insertions(+), 29 deletions(-) diff --git a/android/src/com/mapswithme/maps/discovery/DiscoveryFragment.java b/android/src/com/mapswithme/maps/discovery/DiscoveryFragment.java index ee196ada38..d1f5b18108 100644 --- a/android/src/com/mapswithme/maps/discovery/DiscoveryFragment.java +++ b/android/src/com/mapswithme/maps/discovery/DiscoveryFragment.java @@ -78,7 +78,7 @@ public class DiscoveryFragment extends BaseMwmToolbarFragment implements Discove return; if (ConnectionState.isConnected()) - requestDiscoveryInfoAndInitAdapters(); + NetworkPolicy.checkNetworkPolicy(getFragmentManager(), DiscoveryFragment.this::onNetworkPolicyResult); } }; @@ -183,11 +183,6 @@ public class DiscoveryFragment extends BaseMwmToolbarFragment implements Discove initFoodGallery(); initSearchBasedAdapters(); initCatalogPromoGallery(); - requestDiscoveryInfoAndInitAdapters(); - } - - private void requestDiscoveryInfoAndInitAdapters() - { NetworkPolicy.checkNetworkPolicy(getFragmentManager(), this::onNetworkPolicyResult); } @@ -209,9 +204,10 @@ public class DiscoveryFragment extends BaseMwmToolbarFragment implements Discove { RecyclerView promoRecycler = getGallery(R.id.catalog_promo_recycler); ItemSelectedListener listener = mOnlineMode - ? new CatalogPromoSelectedListener(requireActivity()) - : new ErrorCatalogPromoListener<>(requireActivity(), - this::onNetworkPolicyResult); + ? + new CatalogPromoSelectedListener(requireActivity()) + : new ErrorCatalogPromoListener<>(requireActivity(), + this::onNetworkPolicyResult); GalleryAdapter adapter = mOnlineMode ? Factory.createCatalogPromoLoadingAdapter() : Factory.createCatalogPromoNoConnectionAdapter(listener); @@ -294,6 +290,8 @@ public class DiscoveryFragment extends BaseMwmToolbarFragment implements Discove @Override public void onCatalogPromoResultReceived(@NonNull PromoCityGallery gallery) { + updateViewsVisibility(gallery.getItems(), R.id.catalog_promo_recycler, + R.id.catalog_promo_title); if (gallery.getItems().length == 0) return; diff --git a/android/src/com/mapswithme/maps/gallery/impl/Factory.java b/android/src/com/mapswithme/maps/gallery/impl/Factory.java index 851411a025..62eb44546f 100644 --- a/android/src/com/mapswithme/maps/gallery/impl/Factory.java +++ b/android/src/com/mapswithme/maps/gallery/impl/Factory.java @@ -16,6 +16,7 @@ import com.mapswithme.maps.promo.PromoCityGallery; import com.mapswithme.maps.promo.PromoEntity; import com.mapswithme.maps.search.SearchResult; import com.mapswithme.maps.widget.placepage.PlacePageView; +import com.mapswithme.util.UiUtils; import com.mapswithme.util.statistics.GalleryPlacement; import com.mapswithme.util.statistics.GalleryState; import com.mapswithme.util.statistics.GalleryType; @@ -129,11 +130,8 @@ public class Factory { return new Holders.CatalogErrorHolder(itemView, mItems, getListener()) { - @Override - public void bind(@NonNull Items.Item item) { - super.bind(item); - getButton().setVisibility(View.INVISIBLE); + UiUtils.invisible(itemView, R.id.button); } }; } diff --git a/android/src/com/mapswithme/maps/settings/SettingsPrefsFragment.java b/android/src/com/mapswithme/maps/settings/SettingsPrefsFragment.java index a499973d9f..5e0c4226cf 100644 --- a/android/src/com/mapswithme/maps/settings/SettingsPrefsFragment.java +++ b/android/src/com/mapswithme/maps/settings/SettingsPrefsFragment.java @@ -543,10 +543,13 @@ public class SettingsPrefsFragment extends BaseXmlSettingsFragment if (type == NetworkPolicy.Type.ALWAYS || type == NetworkPolicy.Type.ASK || type == NetworkPolicy.Type.NEVER) - + { Config.setUseMobileDataSettings(type); + } else + { throw new AssertionError("Wrong NetworkPolicy type, value = " + valueStr); + } UiThread.runLater(new Runnable() { diff --git a/android/src/com/mapswithme/maps/widget/placepage/PlacePageView.java b/android/src/com/mapswithme/maps/widget/placepage/PlacePageView.java index 24b68d4088..9510952b34 100644 --- a/android/src/com/mapswithme/maps/widget/placepage/PlacePageView.java +++ b/android/src/com/mapswithme/maps/widget/placepage/PlacePageView.java @@ -1246,12 +1246,12 @@ public class PlacePageView extends NestedScrollView private void processSponsored(@NonNull NetworkPolicy policy) { - boolean hasPromoGallery = mSponsored != null && mSponsored.getType() == Sponsored.TYPE_PROMO_CATALOG; - toggleCatalogPromoGallery(hasPromoGallery); - if (mSponsored == null || mMapObject == null) return; + boolean hasPromoGallery = mSponsored.getType() == Sponsored.TYPE_PROMO_CATALOG; + toggleCatalogPromoGallery(hasPromoGallery); + if (hasPromoGallery && policy.canUseNetwork()) { Promo.INSTANCE.nativeRequestCityGallery(policy, mMapObject.getLat(), mMapObject.getLon(), @@ -1260,7 +1260,7 @@ public class PlacePageView extends NestedScrollView else if (hasPromoGallery) { ErrorCatalogPromoListener listener = - new ErrorCatalogPromoListener<>(getActivity(), this::onNetworkPolicyResult); + new ErrorCatalogPromoListener<>(getActivity(), networkPolicy -> onNetworkPolicyResult(networkPolicy, mMapObject)); com.mapswithme.maps.gallery.GalleryAdapter adapter = Factory.createCatalogPromoNoConnectionAdapter(listener); mCatalogPromoRecycler.setAdapter(adapter); } @@ -1279,11 +1279,11 @@ public class PlacePageView extends NestedScrollView Sponsored.requestInfo(mSponsored, Locale.getDefault().toString(), policy); } - private void onNetworkPolicyResult(@NonNull NetworkPolicy policy) + private void onNetworkPolicyResult(@NonNull NetworkPolicy policy, @NonNull MapObject mapObject) { if (policy.canUseNetwork()) { - Promo.INSTANCE.nativeRequestCityGallery(policy, mMapObject.getLat(), mMapObject.getLon(), + Promo.INSTANCE.nativeRequestCityGallery(policy, mapObject.getLat(), mapObject.getLon(), UTM.UTM_PLACEPAGE_GALLERY); mCatalogPromoRecycler.setAdapter(Factory.createCatalogPromoLoadingAdapter()); } diff --git a/android/src/com/mapswithme/util/NetworkPolicy.java b/android/src/com/mapswithme/util/NetworkPolicy.java index b51df781b7..0343eeff27 100644 --- a/android/src/com/mapswithme/util/NetworkPolicy.java +++ b/android/src/com/mapswithme/util/NetworkPolicy.java @@ -1,13 +1,10 @@ package com.mapswithme.util; -import android.support.annotation.IntDef; import android.support.annotation.NonNull; import android.support.v4.app.FragmentManager; import com.mapswithme.maps.widget.StackedButtonDialogFragment; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; import java.util.concurrent.TimeUnit; public final class NetworkPolicy @@ -19,7 +16,7 @@ public final class NetworkPolicy ALWAYS { @Override - public void apply(@NonNull FragmentManager fragmentManager, + public void check(@NonNull FragmentManager fragmentManager, @NonNull NetworkPolicyListener listener, boolean isDialogAllowed) { boolean nowInRoaming = ConnectionState.isInRoaming(); @@ -35,7 +32,7 @@ public final class NetworkPolicy NEVER { @Override - public void apply(@NonNull FragmentManager fragmentManager, + public void check(@NonNull FragmentManager fragmentManager, @NonNull NetworkPolicyListener listener, boolean isDialogAllowed) { if (isDialogAllowed) @@ -49,7 +46,7 @@ public final class NetworkPolicy NOT_TODAY { @Override - public void apply(@NonNull FragmentManager fragmentManager, + public void check(@NonNull FragmentManager fragmentManager, @NonNull NetworkPolicyListener listener, boolean isDialogAllowed) { if (isDialogAllowed) @@ -62,7 +59,7 @@ public final class NetworkPolicy TODAY { @Override - public void apply(@NonNull FragmentManager fragmentManager, + public void check(@NonNull FragmentManager fragmentManager, @NonNull NetworkPolicyListener listener, boolean isDialogAllowed) { boolean nowInRoaming = ConnectionState.isInRoaming(); @@ -77,7 +74,7 @@ public final class NetworkPolicy NONE; - public void apply(@NonNull FragmentManager fragmentManager, + public void check(@NonNull FragmentManager fragmentManager, @NonNull final NetworkPolicyListener listener, boolean isDialogAllowed) { @@ -107,7 +104,7 @@ public final class NetworkPolicy } Type type = Config.getUseMobileDataSettings(); - type.apply(fragmentManager, listener, isDialogAllowed); + type.check(fragmentManager, listener, isDialogAllowed); } public static void checkNetworkPolicy(@NonNull FragmentManager fragmentManager,