From f10f9cb81c6371063782fbb457dd48a9f7b89edb Mon Sep 17 00:00:00 2001 From: malinajirka Date: Mon, 22 Jul 2019 14:06:07 +0200 Subject: [PATCH 1/2] Move photon url logic from VH to VM --- .../android/ui/posts/PostListFragment.kt | 38 +++++----------- .../ui/posts/PostListItemViewHolder.kt | 21 ++++----- .../android/ui/posts/PostViewHolderConfig.kt | 10 ----- .../ui/posts/adapters/PostListAdapter.kt | 8 ++-- .../ui/reader/utils/ReaderUtilsWrapper.kt | 17 +++++++ .../viewmodel/posts/PostListViewModel.kt | 24 +++++++++- .../viewmodel/posts/PostListViewModelTest.kt | 45 ++++++++----------- 7 files changed, 81 insertions(+), 82 deletions(-) delete mode 100644 WordPress/src/main/java/org/wordpress/android/ui/posts/PostViewHolderConfig.kt create mode 100644 WordPress/src/main/java/org/wordpress/android/ui/reader/utils/ReaderUtilsWrapper.kt diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/PostListFragment.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/PostListFragment.kt index f0032d7989de..73f3c320cea7 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/PostListFragment.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/PostListFragment.kt @@ -26,7 +26,6 @@ import org.wordpress.android.ui.utils.UiHelpers import org.wordpress.android.ui.utils.UiString import org.wordpress.android.util.DisplayUtils import org.wordpress.android.util.NetworkUtils -import org.wordpress.android.util.SiteUtils import org.wordpress.android.util.ToastUtils import org.wordpress.android.util.WPSwipeToRefreshHelper.buildSwipeToRefreshHelper import org.wordpress.android.util.helpers.SwipeToRefreshHelper @@ -63,23 +62,11 @@ class PostListFragment : Fragment() { private lateinit var postListType: PostListType private lateinit var nonNullActivity: FragmentActivity - private lateinit var site: SiteModel - - private val postViewHolderConfig: PostViewHolderConfig by lazy { - val displayWidth = DisplayUtils.getDisplayPixelWidth(context) - val contentSpacing = nonNullActivity.resources.getDimensionPixelSize(R.dimen.content_margin) - PostViewHolderConfig( - photonWidth = displayWidth - contentSpacing * 2, - photonHeight = nonNullActivity.resources.getDimensionPixelSize(R.dimen.reader_featured_image_height), - isPhotonCapable = SiteUtils.isPhotonCapable(site), - imageManager = imageManager - ) - } private val postListAdapter: PostListAdapter by lazy { PostListAdapter( context = nonNullActivity, - postViewHolderConfig = postViewHolderConfig, + imageManager = imageManager, uiHelpers = uiHelpers ) } @@ -89,18 +76,12 @@ class PostListFragment : Fragment() { nonNullActivity = checkNotNull(activity) (nonNullActivity.application as WordPress).component().inject(this) - val site: SiteModel? = if (savedInstanceState == null) { - val nonNullIntent = checkNotNull(nonNullActivity.intent) - nonNullIntent.getSerializableExtra(WordPress.SITE) as SiteModel? - } else { - savedInstanceState.getSerializable(WordPress.SITE) as SiteModel? - } + val nonNullIntent = checkNotNull(nonNullActivity.intent) + val site: SiteModel? = nonNullIntent.getSerializableExtra(WordPress.SITE) as SiteModel? if (site == null) { ToastUtils.showToast(nonNullActivity, R.string.blog_not_found, ToastUtils.Duration.SHORT) nonNullActivity.finish() - } else { - this.site = site } } @@ -136,7 +117,13 @@ class PostListFragment : Fragment() { val postListViewModelConnector = mainViewModel.getPostListViewModelConnector(authorFilter, postListType) viewModel = ViewModelProviders.of(this, viewModelFactory).get(PostListViewModel::class.java) - viewModel.start(postListViewModelConnector) + + val displayWidth = DisplayUtils.getDisplayPixelWidth(context) + val contentSpacing = nonNullActivity.resources.getDimensionPixelSize(R.dimen.content_margin) + viewModel.start( + postListViewModelConnector, photonWidth = displayWidth - contentSpacing * 2, + photonHeight = nonNullActivity.resources.getDimensionPixelSize(R.dimen.reader_featured_image_height) + ) initObservers() } @@ -173,11 +160,6 @@ class PostListFragment : Fragment() { }) } - override fun onSaveInstanceState(outState: Bundle) { - super.onSaveInstanceState(outState) - outState.putSerializable(WordPress.SITE, site) - } - override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View? { val view = inflater.inflate(R.layout.post_list_fragment, container, false) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/PostListItemViewHolder.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/PostListItemViewHolder.kt index f3d51c546ce0..02a6f0ac5215 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/PostListItemViewHolder.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/PostListItemViewHolder.kt @@ -15,9 +15,9 @@ import androidx.annotation.LayoutRes import androidx.core.content.ContextCompat import androidx.recyclerview.widget.RecyclerView import org.wordpress.android.R -import org.wordpress.android.ui.reader.utils.ReaderUtils import org.wordpress.android.ui.utils.UiHelpers import org.wordpress.android.ui.utils.UiString +import org.wordpress.android.util.image.ImageManager import org.wordpress.android.util.image.ImageType import org.wordpress.android.viewmodel.posts.PostListItemAction import org.wordpress.android.viewmodel.posts.PostListItemAction.MoreItem @@ -33,7 +33,7 @@ import org.wordpress.android.widgets.WPTextView sealed class PostListItemViewHolder( @LayoutRes layout: Int, parent: ViewGroup, - private val config: PostViewHolderConfig, + private val imageManager: ImageManager, private val uiHelpers: UiHelpers ) : RecyclerView.ViewHolder(LayoutInflater.from(parent.context).inflate(layout, parent, false)) { private val featuredImageView: ImageView = itemView.findViewById(R.id.image_featured) @@ -52,9 +52,9 @@ sealed class PostListItemViewHolder( class Standard( parent: ViewGroup, - config: PostViewHolderConfig, + imageManager: ImageManager, private val uiHelpers: UiHelpers - ) : PostListItemViewHolder(R.layout.post_list_item, parent, config, uiHelpers) { + ) : PostListItemViewHolder(R.layout.post_list_item, parent, imageManager, uiHelpers) { private val excerptTextView: WPTextView = itemView.findViewById(R.id.excerpt) private val actionButtons: List = listOf( itemView.findViewById(R.id.btn_primary), @@ -95,9 +95,9 @@ sealed class PostListItemViewHolder( class Compact( parent: ViewGroup, - config: PostViewHolderConfig, + imageManager: ImageManager, private val uiHelpers: UiHelpers - ) : PostListItemViewHolder(R.layout.post_list_item_compact, parent, config, uiHelpers) { + ) : PostListItemViewHolder(R.layout.post_list_item_compact, parent, imageManager, uiHelpers) { private val moreButton: ImageButton = itemView.findViewById(R.id.more_button) override fun onBind(item: PostListItemUiState) { @@ -170,15 +170,12 @@ sealed class PostListItemViewHolder( // Suppress blinking as the media upload progresses return } - if (imageUrl == null) { + if (imageUrl.isNullOrBlank()) { featuredImageView.visibility = View.GONE - config.imageManager.cancelRequestAndClearImageView(featuredImageView) + imageManager.cancelRequestAndClearImageView(featuredImageView) } else { - val photonUrl = ReaderUtils.getResizedImageUrl( - imageUrl, config.photonWidth, config.photonHeight, !config.isPhotonCapable - ) featuredImageView.visibility = View.VISIBLE - config.imageManager.load(featuredImageView, ImageType.PHOTO, photonUrl, ScaleType.CENTER_CROP) + imageManager.load(featuredImageView, ImageType.PHOTO, imageUrl, ScaleType.CENTER_CROP) } loadedFeaturedImgUrl = imageUrl } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/PostViewHolderConfig.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/PostViewHolderConfig.kt deleted file mode 100644 index b4a11f69579b..000000000000 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/PostViewHolderConfig.kt +++ /dev/null @@ -1,10 +0,0 @@ -package org.wordpress.android.ui.posts - -import org.wordpress.android.util.image.ImageManager - -class PostViewHolderConfig( - val photonWidth: Int, - val photonHeight: Int, - val isPhotonCapable: Boolean, - val imageManager: ImageManager -) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/posts/adapters/PostListAdapter.kt b/WordPress/src/main/java/org/wordpress/android/ui/posts/adapters/PostListAdapter.kt index 56ae9300d149..dba0a98b973d 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/posts/adapters/PostListAdapter.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/posts/adapters/PostListAdapter.kt @@ -14,8 +14,8 @@ import org.wordpress.android.ui.posts.PostListItemViewHolder import org.wordpress.android.ui.posts.PostListViewLayoutType import org.wordpress.android.ui.posts.PostListViewLayoutType.COMPACT import org.wordpress.android.ui.posts.PostListViewLayoutType.STANDARD -import org.wordpress.android.ui.posts.PostViewHolderConfig import org.wordpress.android.ui.utils.UiHelpers +import org.wordpress.android.util.image.ImageManager import org.wordpress.android.viewmodel.posts.PostListItemProgressBar import org.wordpress.android.viewmodel.posts.PostListItemType import org.wordpress.android.viewmodel.posts.PostListItemType.EndListIndicatorItem @@ -30,7 +30,7 @@ private const val VIEW_TYPE_LOADING_COMPACT = 4 class PostListAdapter( context: Context, - private val postViewHolderConfig: PostViewHolderConfig, + private val imageManager: ImageManager, private val uiHelpers: UiHelpers ) : PagedListAdapter(PostListDiffItemCallback) { private val layoutInflater: LayoutInflater = LayoutInflater.from(context) @@ -69,10 +69,10 @@ class PostListAdapter( LoadingViewHolder(view) } VIEW_TYPE_POST -> { - PostListItemViewHolder.Standard(parent, postViewHolderConfig, uiHelpers) + PostListItemViewHolder.Standard(parent, imageManager, uiHelpers) } VIEW_TYPE_POST_COMPACT -> { - PostListItemViewHolder.Compact(parent, postViewHolderConfig, uiHelpers) + PostListItemViewHolder.Compact(parent, imageManager, uiHelpers) } else -> { // Fail fast if a new view type is added so the we can handle it diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/utils/ReaderUtilsWrapper.kt b/WordPress/src/main/java/org/wordpress/android/ui/reader/utils/ReaderUtilsWrapper.kt new file mode 100644 index 000000000000..64a6c86978e2 --- /dev/null +++ b/WordPress/src/main/java/org/wordpress/android/ui/reader/utils/ReaderUtilsWrapper.kt @@ -0,0 +1,17 @@ +package org.wordpress.android.ui.reader.utils + +import dagger.Reusable +import javax.inject.Inject + +/** + * Injectable wrapper around ReaderUtils. + * + * ReaderUtils interface is consisted of static methods, which make the client code difficult to test/mock. Main purpose of + * this wrapper is to make testing easier. + * + */ +@Reusable +class ReaderUtilsWrapper @Inject constructor() { + fun getResizedImageUrl(imageUrl: String?, width: Int, height: Int, isPrivate: Boolean): String? = + ReaderUtils.getResizedImageUrl(imageUrl, width, height, isPrivate) +} diff --git a/WordPress/src/main/java/org/wordpress/android/viewmodel/posts/PostListViewModel.kt b/WordPress/src/main/java/org/wordpress/android/viewmodel/posts/PostListViewModel.kt index 6d755da25571..17dc48bb125f 100644 --- a/WordPress/src/main/java/org/wordpress/android/viewmodel/posts/PostListViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/viewmodel/posts/PostListViewModel.kt @@ -34,6 +34,7 @@ import org.wordpress.android.ui.posts.AuthorFilterSelection.ME import org.wordpress.android.ui.posts.PostListType.SEARCH import org.wordpress.android.ui.posts.PostUtils import org.wordpress.android.ui.posts.trackPostListAction +import org.wordpress.android.ui.reader.utils.ReaderUtilsWrapper import org.wordpress.android.ui.uploads.UploadStarter import org.wordpress.android.util.AppLog import org.wordpress.android.util.NetworkUtilsWrapper @@ -47,6 +48,7 @@ import org.wordpress.android.viewmodel.posts.PostListItemIdentifier.LocalPostId import org.wordpress.android.viewmodel.posts.PostListItemType.PostListItemUiState import javax.inject.Inject import javax.inject.Named +import kotlin.properties.Delegates typealias PagedPostList = PagedList @@ -63,6 +65,7 @@ class PostListViewModel @Inject constructor( private val listItemUiStateHelper: PostListItemUiStateHelper, private val networkUtilsWrapper: NetworkUtilsWrapper, private val uploadStarter: UploadStarter, + private val readerUtilsWrapper: ReaderUtilsWrapper, @Named(UI_THREAD) private val uiDispatcher: CoroutineDispatcher, @Named(BG_THREAD) private val bgDispatcher: CoroutineDispatcher, connectionStatus: LiveData @@ -73,6 +76,9 @@ class PostListViewModel @Inject constructor( private var isStarted: Boolean = false private lateinit var connector: PostListViewModelConnector + private var photonWidth by Delegates.notNull() + private var photonHeight by Delegates.notNull() + private var scrollToLocalPostId: LocalPostId? = null private val _scrollToPosition = SingleLiveEvent() @@ -113,10 +119,16 @@ class PostListViewModel @Inject constructor( private val lifecycleRegistry = LifecycleRegistry(this) override fun getLifecycle(): Lifecycle = lifecycleRegistry - fun start(postListViewModelConnector: PostListViewModelConnector) { + fun start( + postListViewModelConnector: PostListViewModelConnector, + photonWidth: Int, + photonHeight: Int + ) { if (isStarted) { return } + this.photonHeight = photonHeight + this.photonWidth = photonWidth connector = postListViewModelConnector if (connector.postListType != SEARCH) { @@ -337,7 +349,7 @@ class PostListViewModel @Inject constructor( unhandledConflicts = connector.doesPostHaveUnhandledConflict(post), capabilitiesToPublish = connector.site.hasCapabilityPublishPosts, statsSupported = isStatsSupported, - featuredImageUrl = connector.getFeaturedImageUrl(post.featuredImageId), + featuredImageUrl = convertToPhotonUrlIfPossible(connector.getFeaturedImageUrl(post.featuredImageId)), formattedDate = PostUtils.getFormattedDate(post), performingCriticalAction = connector.postActionHandler.isPerformingCriticalAction(LocalId(post.id)), onAction = { postModel, buttonType, statEvent -> @@ -354,4 +366,12 @@ class PostListViewModel @Inject constructor( fetchFirstPage() } } + + private fun convertToPhotonUrlIfPossible(featuredImageUrl: String?): String? = + readerUtilsWrapper.getResizedImageUrl( + featuredImageUrl, + photonWidth, + photonHeight, + !SiteUtils.isPhotonCapable(connector.site) + ) } diff --git a/WordPress/src/test/java/org/wordpress/android/viewmodel/posts/PostListViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/viewmodel/posts/PostListViewModelTest.kt index 07a3d6c81d12..83c37c65a486 100644 --- a/WordPress/src/test/java/org/wordpress/android/viewmodel/posts/PostListViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/viewmodel/posts/PostListViewModelTest.kt @@ -22,6 +22,7 @@ import org.wordpress.android.ui.posts.PostListType.DRAFTS import org.wordpress.android.ui.posts.PostListType.SEARCH import org.wordpress.android.ui.uploads.UploadStarter +private const val DEFAULT_PHOTON_DIMENSIONS = -9 class PostListViewModelTest : BaseUnitTest() { @Mock private lateinit var site: SiteModel @Mock private lateinit var uploadStarter: UploadStarter @@ -34,33 +35,16 @@ class PostListViewModelTest : BaseUnitTest() { val listStore = mock() val postList = mock>() - whenever( - postList.listError - ).thenReturn(mock()) - - whenever( - postList.isFetchingFirstPage - ).thenReturn(mock()) - - whenever( - postList.isEmpty - ).thenReturn(mock()) - - whenever( - postList.data - ).thenReturn(mock()) - - whenever( - postList.isLoadingMore - ).thenReturn(mock()) - - whenever( - listStore.getList( + whenever(postList.listError).thenReturn(mock()) + whenever(postList.isFetchingFirstPage).thenReturn(mock()) + whenever(postList.isEmpty).thenReturn(mock()) + whenever(postList.data).thenReturn(mock()) + whenever(postList.isLoadingMore).thenReturn(mock()) + whenever(listStore.getList( any(), any(), any() - ) - ).thenReturn(postList) + )).thenReturn(postList) viewModel = PostListViewModel( dispatcher = mock(), @@ -70,6 +54,7 @@ class PostListViewModelTest : BaseUnitTest() { listItemUiStateHelper = mock(), networkUtilsWrapper = mock(), uploadStarter = uploadStarter, + readerUtilsWrapper = mock(), connectionStatus = mock(), uiDispatcher = TEST_DISPATCHER, bgDispatcher = TEST_DISPATCHER @@ -78,7 +63,11 @@ class PostListViewModelTest : BaseUnitTest() { @Test fun `when swiping to refresh, it uploads all local drafts`() { - viewModel.start(createPostListViewModelConnector(site = site, postListType = DRAFTS)) + viewModel.start( + createPostListViewModelConnector(site = site, postListType = DRAFTS), + DEFAULT_PHOTON_DIMENSIONS, + DEFAULT_PHOTON_DIMENSIONS + ) // When viewModel.swipeToRefresh() @@ -89,7 +78,11 @@ class PostListViewModelTest : BaseUnitTest() { @Test fun `empty search query should show search prompt`() { - viewModel.start(createPostListViewModelConnector(site = site, postListType = SEARCH)) + viewModel.start( + createPostListViewModelConnector(site = site, postListType = SEARCH), + DEFAULT_PHOTON_DIMENSIONS, + DEFAULT_PHOTON_DIMENSIONS + ) val emptyViewStateResults = mutableListOf() From 404ee70b3b7f5e578cc3636494de136cadbf05ef Mon Sep 17 00:00:00 2001 From: malinajirka Date: Mon, 22 Jul 2019 15:12:32 +0200 Subject: [PATCH 2/2] Fix line length --- .../org/wordpress/android/viewmodel/posts/PostListViewModel.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/WordPress/src/main/java/org/wordpress/android/viewmodel/posts/PostListViewModel.kt b/WordPress/src/main/java/org/wordpress/android/viewmodel/posts/PostListViewModel.kt index 17dc48bb125f..d9cea21ae24f 100644 --- a/WordPress/src/main/java/org/wordpress/android/viewmodel/posts/PostListViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/viewmodel/posts/PostListViewModel.kt @@ -349,7 +349,8 @@ class PostListViewModel @Inject constructor( unhandledConflicts = connector.doesPostHaveUnhandledConflict(post), capabilitiesToPublish = connector.site.hasCapabilityPublishPosts, statsSupported = isStatsSupported, - featuredImageUrl = convertToPhotonUrlIfPossible(connector.getFeaturedImageUrl(post.featuredImageId)), + featuredImageUrl = + convertToPhotonUrlIfPossible(connector.getFeaturedImageUrl(post.featuredImageId)), formattedDate = PostUtils.getFormattedDate(post), performingCriticalAction = connector.postActionHandler.isPerformingCriticalAction(LocalId(post.id)), onAction = { postModel, buttonType, statEvent ->