From f9cda01b1822a98ef1757d38dcf7fa0390c2b5a1 Mon Sep 17 00:00:00 2001 From: Dmitry Donskoy Date: Tue, 19 Jun 2018 19:35:31 +0300 Subject: [PATCH] [android] Fixed review notes --- .../maps/bookmarks/data/BookmarkManager.cpp | 3 +- android/res/layout/fragment_bookmark_list.xml | 6 ++-- .../res/layout/item_category_description.xml | 6 ++-- android/res/layout/item_category_title.xml | 9 +++-- .../BaseBookmarkCategoriesFragment.java | 2 +- .../bookmarks/BookmarkCategoriesAdapter.java | 7 ++-- .../maps/bookmarks/BookmarkListAdapter.java | 4 ++- .../bookmarks/BookmarksCatalogFragment.java | 15 ++++---- .../maps/bookmarks/BookmarksListFragment.java | 8 ++--- .../CachedBookmarkCategoriesFragment.java | 6 ++-- .../mapswithme/maps/bookmarks/Holders.java | 35 ++++++++++--------- .../maps/bookmarks/data/BookmarkCategory.java | 2 ++ .../maps/bookmarks/data/BookmarkManager.java | 7 ++-- .../util/SharedPropertiesUtils.java | 6 ++-- 14 files changed, 62 insertions(+), 54 deletions(-) diff --git a/android/jni/com/mapswithme/maps/bookmarks/data/BookmarkManager.cpp b/android/jni/com/mapswithme/maps/bookmarks/data/BookmarkManager.cpp index 691237ae5b..a6940bcee2 100644 --- a/android/jni/com/mapswithme/maps/bookmarks/data/BookmarkManager.cpp +++ b/android/jni/com/mapswithme/maps/bookmarks/data/BookmarkManager.cpp @@ -81,7 +81,8 @@ void PrepareClassRefs(JNIEnv * env) // boolean fromCatalog, // boolean isVisible) g_bookmarkCategoryConstructor = - jni::GetConstructorID(env, g_bookmarkCategoryClass, "(JLjava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;IIZZ)V"); + jni::GetConstructorID(env, g_bookmarkCategoryClass, "(JLjava/lang/String;Ljava/lang/String;" + "Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;IIZZ)V"); } void OnAsyncLoadingStarted(JNIEnv * env) diff --git a/android/res/layout/fragment_bookmark_list.xml b/android/res/layout/fragment_bookmark_list.xml index 609fb154f5..69245fba74 100644 --- a/android/res/layout/fragment_bookmark_list.xml +++ b/android/res/layout/fragment_bookmark_list.xml @@ -21,8 +21,6 @@ android:paddingRight="@dimen/margin_double_and_half" android:paddingTop="@dimen/placeholder_margin_top" mapsme:imgSrcDefault="@drawable/img_empty_bookmarks" - mapsme:titleDefault="@string/empty_bookmarks_list_title" - mapsme:subTitleDefault="@string/empty_bookmarks_list_subtitle"/> - + mapsme:titleDefault="@string/bookmarks_empty_list_title" + mapsme:subTitleDefault="@string/bookmarks_empty_list_message"/> - diff --git a/android/res/layout/item_category_description.xml b/android/res/layout/item_category_description.xml index f4f1378dff..642ca9f190 100644 --- a/android/res/layout/item_category_description.xml +++ b/android/res/layout/item_category_description.xml @@ -33,10 +33,10 @@ android:id="@+id/more_btn" android:layout_width="wrap_content" android:layout_height="wrap_content" - android:layout_gravity="left|bottom" - android:gravity="left|top" + android:layout_gravity="start|bottom" + android:gravity="start|top" android:textColor="?attr/colorAccent" - android:text="@string/more" + android:text="@string/category_desc_more" android:background="@android:color/transparent"/> diff --git a/android/res/layout/item_category_title.xml b/android/res/layout/item_category_title.xml index 8f4873302f..ab3186f0dc 100644 --- a/android/res/layout/item_category_title.xml +++ b/android/res/layout/item_category_title.xml @@ -5,10 +5,13 @@ xmlns:android="http://schemas.android.com/apk/res/android" android:layout_width="match_parent" android:layout_height="@dimen/height_item_oneline" - android:gravity="left|center_vertical" - android:paddingLeft="@dimen/margin_base" + android:gravity="start|center_vertical" android:paddingTop="@dimen/margin_half" android:textAppearance="@style/MwmTextAppearance.Body3" android:fontFamily="@string/robotoMedium" android:text="@string/bookmarks" - tools:targetApi="jelly_bean"/> + tools:targetApi="jelly_bean" + android:paddingEnd="@dimen/margin_base" + android:paddingRight="@dimen/margin_base" + android:paddingStart="@dimen/margin_base" + android:paddingLeft="@dimen/margin_base"/> diff --git a/android/src/com/mapswithme/maps/bookmarks/BaseBookmarkCategoriesFragment.java b/android/src/com/mapswithme/maps/bookmarks/BaseBookmarkCategoriesFragment.java index b82525ab5e..0a83c7f48f 100644 --- a/android/src/com/mapswithme/maps/bookmarks/BaseBookmarkCategoriesFragment.java +++ b/android/src/com/mapswithme/maps/bookmarks/BaseBookmarkCategoriesFragment.java @@ -274,7 +274,7 @@ public abstract class BaseBookmarkCategoriesFragment extends BaseMwmRecyclerFrag } @NonNull - private Intent makeBookmarksListIntent(BookmarkCategory category) + private Intent makeBookmarksListIntent(@NonNull BookmarkCategory category) { return new Intent(getActivity(), BookmarkListActivity.class) .putExtra(BookmarksListFragment.EXTRA_CATEGORY, category); diff --git a/android/src/com/mapswithme/maps/bookmarks/BookmarkCategoriesAdapter.java b/android/src/com/mapswithme/maps/bookmarks/BookmarkCategoriesAdapter.java index 8fcbf3fd52..2e52596ce1 100644 --- a/android/src/com/mapswithme/maps/bookmarks/BookmarkCategoriesAdapter.java +++ b/android/src/com/mapswithme/maps/bookmarks/BookmarkCategoriesAdapter.java @@ -152,14 +152,15 @@ public class BookmarkCategoriesAdapter extends BaseBookmarkCategoryAdapter { + @NonNull private final BookmarkCategory mCategory; // view types @@ -50,7 +52,7 @@ public class BookmarkListAdapter extends RecyclerView.Adapter(frag); } @@ -134,9 +135,8 @@ public class BookmarksCatalogFragment extends BaseWebViewMwmFragment return; } - frag.mWebView.setVisibility(View.VISIBLE); - frag.mProgressView.setVisibility(View.GONE); - frag.mRetryBtn.setVisibility(View.GONE); + UiUtils.show(frag.mWebView); + UiUtils.hide(frag.mProgressView, frag.mRetryBtn); } @Override @@ -148,9 +148,8 @@ public class BookmarksCatalogFragment extends BaseWebViewMwmFragment if ((frag = mReference.get()) == null) return; - frag.mWebView.setVisibility(View.GONE); - frag.mProgressView.setVisibility(View.GONE); - frag.mRetryBtn.setVisibility(View.VISIBLE); + UiUtils.show(frag.mRetryBtn); + UiUtils.hide(frag.mWebView, frag.mProgressView); } private void retry() diff --git a/android/src/com/mapswithme/maps/bookmarks/BookmarksListFragment.java b/android/src/com/mapswithme/maps/bookmarks/BookmarksListFragment.java index ce5c19f65a..1715fdd45f 100644 --- a/android/src/com/mapswithme/maps/bookmarks/BookmarksListFragment.java +++ b/android/src/com/mapswithme/maps/bookmarks/BookmarksListFragment.java @@ -28,7 +28,7 @@ import com.mapswithme.maps.widget.placepage.Sponsored; import com.mapswithme.maps.widget.recycler.RecyclerClickListener; import com.mapswithme.maps.widget.recycler.RecyclerLongClickListener; import com.mapswithme.util.BottomSheetHelper; -import com.mapswithme.util.Utils; +import com.mapswithme.util.UiUtils; import com.mapswithme.util.sharing.ShareOption; import com.mapswithme.util.sharing.SharingHelper; @@ -70,9 +70,9 @@ public class BookmarksListFragment extends BaseMwmRecyclerFragment super.onViewCreated(view, savedInstanceState); configureAdapter(); setHasOptionsMenu(true); - boolean isEmptyDataSet = getAdapter().getItemCount() == 0; - getRecyclerView().setVisibility(isEmptyDataSet ? View.GONE : View.VISIBLE); - showPlaceholder(isEmptyDataSet); + boolean isEmpty = getAdapter().getItemCount() == 0; + UiUtils.showIf(!isEmpty, getRecyclerView()); + showPlaceholder(isEmpty); ActionBar bar = ((AppCompatActivity) getActivity()).getSupportActionBar(); if (bar != null) bar.setTitle(mCategory.getName()); diff --git a/android/src/com/mapswithme/maps/bookmarks/CachedBookmarkCategoriesFragment.java b/android/src/com/mapswithme/maps/bookmarks/CachedBookmarkCategoriesFragment.java index 68ec820bbd..152bde9c7f 100644 --- a/android/src/com/mapswithme/maps/bookmarks/CachedBookmarkCategoriesFragment.java +++ b/android/src/com/mapswithme/maps/bookmarks/CachedBookmarkCategoriesFragment.java @@ -12,17 +12,19 @@ import com.mapswithme.maps.R; import com.mapswithme.maps.bookmarks.data.BookmarkCategory; import com.mapswithme.maps.bookmarks.data.BookmarkManager; import com.mapswithme.util.SharedPropertiesUtils; -import com.mapswithme.util.StorageUtils; import com.mapswithme.util.UiUtils; import com.mapswithme.util.sharing.TargetUtils; public class CachedBookmarkCategoriesFragment extends BaseBookmarkCategoriesFragment implements BookmarkManager.BookmarksCatalogListener { + @SuppressWarnings("NullableProblems") @NonNull private ViewGroup mEmptyViewContainer; + @SuppressWarnings("NullableProblems") @NonNull private View mPayloadContainer; + @SuppressWarnings("NullableProblems") @NonNull private View mProgressContainer; @@ -38,7 +40,7 @@ public class CachedBookmarkCategoriesFragment extends BaseBookmarkCategoriesFrag View closeHeaderBtn = root.findViewById(R.id.header_close); closeHeaderBtn.setOnClickListener(new CloseHeaderClickListener()); boolean isClosed = SharedPropertiesUtils.isCatalogCategoriesHeaderClosed(getContext()); - closeHeaderBtn.setVisibility(isClosed ? View.GONE : View.VISIBLE); + UiUtils.showIf(!isClosed, closeHeaderBtn); return root; } diff --git a/android/src/com/mapswithme/maps/bookmarks/Holders.java b/android/src/com/mapswithme/maps/bookmarks/Holders.java index b6edb1089f..13368f59b2 100644 --- a/android/src/com/mapswithme/maps/bookmarks/Holders.java +++ b/android/src/com/mapswithme/maps/bookmarks/Holders.java @@ -221,7 +221,7 @@ public class Holders abstract void bind(int position); - static boolean isSectionEmpty(BookmarkCategory category, @Section int section) + static boolean isSectionEmpty(@NonNull BookmarkCategory category, @Section int section) { switch (section) { @@ -236,7 +236,7 @@ public class Holders } } - static int getSectionForPosition(BookmarkCategory category, int position) + static int getSectionForPosition(@NonNull BookmarkCategory category, int position) { if (position == getDescSectionPosition(category)) return SECTION_DESC; @@ -248,39 +248,40 @@ public class Holders throw new IllegalArgumentException("There is no section in position " + position); } - static int getDescSectionPosition(BookmarkCategory category) + static int getDescSectionPosition(@NonNull BookmarkCategory category) { if (isSectionEmpty(category, SECTION_DESC)) - return -1; + return RecyclerView.NO_POSITION; return 0; } - static int getTracksSectionPosition(BookmarkCategory category) + static int getTracksSectionPosition(@NonNull BookmarkCategory category) { if (isSectionEmpty(category, SECTION_TRACKS)) - return -1; + return RecyclerView.NO_POSITION; return getDescItemCount(category); } - static int getBookmarksSectionPosition(BookmarkCategory category) + static int getBookmarksSectionPosition(@NonNull BookmarkCategory category) { if (isSectionEmpty(category, SECTION_BMKS)) - return -1; + return RecyclerView.NO_POSITION; int beforeCurrentSectionItemsCount = getTracksSectionPosition(category); - return (beforeCurrentSectionItemsCount == -1 ? getDescItemCount(category) - : beforeCurrentSectionItemsCount) + return (beforeCurrentSectionItemsCount == RecyclerView.NO_POSITION + ? getDescItemCount(category) + : beforeCurrentSectionItemsCount) + getTrackItemCount(category); } - private static int getTrackItemCount(BookmarkCategory category) + private static int getTrackItemCount(@NonNull BookmarkCategory category) { return category.getTracksCount() + (isSectionEmpty(category, SECTION_TRACKS) ? 0 : 1); } - static int getDescItemCount(BookmarkCategory category) + static int getDescItemCount(@NonNull BookmarkCategory category) { return isSectionEmpty(category, SECTION_DESC) ? 0 : /* section header */ 1 + /* non empty desc */ 1; } @@ -298,7 +299,7 @@ public class Holders mView.setOnLongClickListener(v -> onOpenActionMenu(v, listener)); } - boolean onOpenActionMenu(View v, @Nullable RecyclerLongClickListener listener) + boolean onOpenActionMenu(@NonNull View v, @Nullable RecyclerLongClickListener listener) { if (listener != null) listener.onLongItemClick(v, getAdapterPosition()); @@ -317,9 +318,9 @@ public class Holders @NonNull private final View mMore; - BookmarkViewHolder(@NonNull View itemView, BookmarkCategory categoryId) + BookmarkViewHolder(@NonNull View itemView, @NonNull BookmarkCategory category) { - super(itemView, categoryId); + super(itemView, category); mIcon = itemView.findViewById(R.id.iv__bookmark_color); mName = itemView.findViewById(R.id.tv__bookmark_name); mDistance = itemView.findViewById(R.id.tv__bookmark_distance); @@ -352,7 +353,7 @@ public class Holders mIcon.setImageResource(bookmark.getIcon().getSelectedResId()); } - static int calculateBookmarkPosition(BookmarkCategory category, int position) + static int calculateBookmarkPosition(@NonNull BookmarkCategory category, int position) { // Since bookmarks are always below tracks and header we should take it into account // during the bookmark's position calculation. @@ -448,7 +449,7 @@ public class Holders mAuthor = itemView.findViewById(R.id.author); } - private void onMoreBtnClicked(View v) + private void onMoreBtnClicked(@NonNull View v) { int lineCount = calcLineCount(mDescText, mCategory.getDescription()); mDescText.setLines(lineCount); diff --git a/android/src/com/mapswithme/maps/bookmarks/data/BookmarkCategory.java b/android/src/com/mapswithme/maps/bookmarks/data/BookmarkCategory.java index d6eb7d57d9..d4e741e637 100644 --- a/android/src/com/mapswithme/maps/bookmarks/data/BookmarkCategory.java +++ b/android/src/com/mapswithme/maps/bookmarks/data/BookmarkCategory.java @@ -280,7 +280,9 @@ public class BookmarkCategory implements Parcelable PRIVATE(BookmarksPageFactory.PRIVATE, FilterStrategy.PredicativeStrategy.makePrivateInstance()), CATALOG(BookmarksPageFactory.CATALOG, FilterStrategy.PredicativeStrategy.makeCatalogInstance()); + @NonNull private BookmarksPageFactory mFactory; + @NonNull private FilterStrategy mFilterStrategy; Type(@NonNull BookmarksPageFactory pageFactory, @NonNull FilterStrategy filterStrategy) diff --git a/android/src/com/mapswithme/maps/bookmarks/data/BookmarkManager.java b/android/src/com/mapswithme/maps/bookmarks/data/BookmarkManager.java index 391d8073b7..e677fbce7a 100644 --- a/android/src/com/mapswithme/maps/bookmarks/data/BookmarkManager.java +++ b/android/src/com/mapswithme/maps/bookmarks/data/BookmarkManager.java @@ -283,13 +283,12 @@ public enum BookmarkManager /** * @return total count - tracks + bookmarks - * @param catId - * should be used BookmarkCategory.size() - without JNI method call overhead + * @param category */ @Deprecated - public int getCategorySize(BookmarkCategory catId) + public int getCategorySize(@NonNull BookmarkCategory category) { - return nativeGetBookmarksCount(catId.getId()) + nativeGetTracksCount(catId.getId()); + return nativeGetBookmarksCount(category.getId()) + nativeGetTracksCount(category.getId()); } public int getCategoriesCount() { return nativeGetCategoriesCount(); } diff --git a/android/src/com/mapswithme/util/SharedPropertiesUtils.java b/android/src/com/mapswithme/util/SharedPropertiesUtils.java index 85545d49c5..6e120a7c6f 100644 --- a/android/src/com/mapswithme/util/SharedPropertiesUtils.java +++ b/android/src/com/mapswithme/util/SharedPropertiesUtils.java @@ -16,7 +16,7 @@ public final class SharedPropertiesUtils private static final String PREFS_SHOW_EMULATE_BAD_STORAGE_SETTING = "ShowEmulateBadStorageSetting"; private static final String PREFS_BACKUP_WIDGET_EXPANDED = "BackupWidgetExpanded"; private static final String PREFS_WHATS_NEW_TITLE_CONCATENATION = "WhatsNewTitleConcatenation"; - private static final String PREFS_CATALOG_CATEGORIES_HEADER_CLOSED = "catalog_categories_header_closed"; + private static final String PREFS_CATALOG_CATEGORIES_HEADER_CLOSED = "CatalogCategoriesHeaderClosed"; private static final SharedPreferences PREFS = PreferenceManager.getDefaultSharedPreferences(MwmApplication.get()); @@ -79,13 +79,13 @@ public final class SharedPropertiesUtils PREFS.edit().putString(PREFS_WHATS_NEW_TITLE_CONCATENATION, concatenation).apply(); } - public static boolean isCatalogCategoriesHeaderClosed(Context context) + public static boolean isCatalogCategoriesHeaderClosed(@NonNull Context context) { return MwmApplication.prefs(context) .getBoolean(PREFS_CATALOG_CATEGORIES_HEADER_CLOSED, false); } - public static void setCatalogCategoriesHeaderClosed(Context context, boolean value) + public static void setCatalogCategoriesHeaderClosed(@NonNull Context context, boolean value) { MwmApplication.prefs(context) .edit()