From 83403ebb581a6386c1a98a1c9a14a78ed301b062 Mon Sep 17 00:00:00 2001 From: Christophe Beyls Date: Fri, 31 May 2024 13:28:01 +0200 Subject: [PATCH] Clear View-related references in Fragments and use viewLifecycleOwner when applicable (#4470) This pull request has main 2 goals related to improving the handling of View lifecycles in Fragments: - **Use viewLifecycleOwner when applicable**: every coroutine touching Views in Fragments must be launched in the `coroutinescope` of `viewLifecycleOwner` to avoid the following issues: 1. The code will crash if it references a View binding that is no more available and keeps running after the Fragment view hierarchy has been destroyed. 2. The code will leak Views if it references Views from its parent scope after the Fragment view hierarchy has been destroyed. 3. Multiple instances of the same coroutine will run at the same time, if the coroutine is launched in `onViewCreated()` from the wrong scope and the view hierarchy is destroyed then re-created in the same Fragment instance. - **Clear View-related references in Fragments**: it is an error to keep a reference to Views or any other class containing View references in Fragments after `onDestroyView()`. It creates a memory leak when the Fragment is added to the back stack or is temporarily detached. A typical object that leaks Views is a RecyclerView's Adapter: when the adapter is set on the RecyclerView, the RecyclerView registers itself as a listener to the Adapter and the Adapter now contains a reference to the RecyclerView that is not automatically cleared. It is thus recommended to clear all these view references one way or another, even if the Fragment is currently not part of a scenario where it is detached or added to a back stack. In general, having a `lateinit var` not related to Dagger dependency injection in a Fragment is a code smell and should be avoided: - If that `lateinit var` is related to storing something View-related, it must be removed if possible or made nullable and set to `null` in `onDestroyView()`. - If that `lateinit var` is meant to store fragment arguments, it can be turned into a `val by lazy{}`. - If that `lateinit var` is related to fetching some information from the Activity, it can be turned into a `val` with no backing field that will simply call the activity when accessed. There is no need to store the value in the Fragment. When possible, View-related values must not be stored as Fragment fields: all views should be accessed only in `onViewCreated()` and passed as arguments to various listeners down the chain. However, it's still required to use nullable fields **when the Fragment exposes public methods that are called directly by an external entity**, and these methods use the View-related value. Since the Fragment has no control over when the external entity will call these public methods, the field must never assumed to be non-null and null checks must be added for every call. Note that exposing public methods on a Fragment to be called directly is an antipattern, but switching to a different architecture is out of scope of this pull request. - Use `viewLifecycleOwner` in Fragments where applicable. - Remove view-related fields and instead declare them in `onViewCreated()` when possible. - When not possible, declare view-related fields as nullable and set them to `null` in `onDestroyView()`. - Pass non-null View-related fields as arguments when possible, to not rely on the nullable Fragment field. - Replace `lateinit var` containing an Activity-related value with `val` accessing the Activity directly on demand. - Remove some unused fragment fields. - Replace `onCreateView()` with passing the layout id as Fragment constructor argument when possible. - Replace `isAdded` checks with `view != null`. A Fragment being added to an Activity doesn't mean it has a View hierarchy (it may be detached and invisible). - Remove `mediaPlayerListener` field in `ViewVideoFragment` and turn it into a local variable. It is then passed into a `DefaultLifecycleObserver` that replaces the `onStart()`/`onStop()` overrides and is unregistered automatically when the view hierarchy is destroyed. --- .../tusky/AccountsInListFragment.kt | 19 ++-- .../account/list/ListSelectionFragment.kt | 21 +--- .../account/media/AccountMediaFragment.kt | 13 ++- .../accountlist/AccountListFragment.kt | 67 ++++++++----- .../conversation/ConversationsFragment.kt | 59 ++++++------ .../domainblocks/DomainBlocksFragment.kt | 2 +- .../notifications/NotificationsFragment.kt | 74 +++++++------- .../fragments/ReportStatusesFragment.kt | 33 ++++--- .../search/fragments/SearchFragment.kt | 43 ++++++--- .../fragments/SearchStatusesFragment.kt | 52 +++++----- .../components/timeline/TimelineFragment.kt | 96 ++++++++++--------- .../trending/TrendingTagsFragment.kt | 45 +++++---- .../viewthread/ViewThreadFragment.kt | 74 +++++++------- .../keylesspalace/tusky/fragment/SFragment.kt | 39 ++++---- .../tusky/fragment/ViewImageFragment.kt | 12 +-- .../tusky/fragment/ViewVideoFragment.kt | 69 ++++++------- 16 files changed, 369 insertions(+), 349 deletions(-) diff --git a/app/src/main/java/com/keylesspalace/tusky/AccountsInListFragment.kt b/app/src/main/java/com/keylesspalace/tusky/AccountsInListFragment.kt index 8676e43d3..4e6e42091 100644 --- a/app/src/main/java/com/keylesspalace/tusky/AccountsInListFragment.kt +++ b/app/src/main/java/com/keylesspalace/tusky/AccountsInListFragment.kt @@ -49,7 +49,7 @@ import kotlinx.coroutines.launch private typealias AccountInfo = Pair @AndroidEntryPoint -class AccountsInListFragment : DialogFragment() { +class AccountsInListFragment : DialogFragment(R.layout.fragment_accounts_in_list) { @Inject lateinit var preferences: SharedPreferences @@ -59,8 +59,6 @@ class AccountsInListFragment : DialogFragment() { private lateinit var listId: String private lateinit var listName: String - private val adapter = Adapter() - private val searchAdapter = SearchAdapter() private val radius by unsafeLazy { resources.getDimensionPixelSize(R.dimen.avatar_radius_48dp) } @@ -85,15 +83,10 @@ class AccountsInListFragment : DialogFragment() { } } - override fun onCreateView( - inflater: LayoutInflater, - container: ViewGroup?, - savedInstanceState: Bundle? - ): View? { - return inflater.inflate(R.layout.fragment_accounts_in_list, container, false) - } - override fun onViewCreated(view: View, savedInstanceState: Bundle?) { + val adapter = Adapter() + val searchAdapter = SearchAdapter() + binding.accountsRecycler.layoutManager = LinearLayoutManager(view.context) binding.accountsRecycler.adapter = adapter @@ -109,7 +102,7 @@ class AccountsInListFragment : DialogFragment() { onFailure = { handleError(it) } ) - setupSearchView(state) + setupSearchView(searchAdapter, state) } } @@ -130,7 +123,7 @@ class AccountsInListFragment : DialogFragment() { }) } - private fun setupSearchView(state: State) { + private fun setupSearchView(searchAdapter: SearchAdapter, state: State) { if (state.searchResult == null) { searchAdapter.submitList(listOf()) binding.accountsSearchRecycler.hide() diff --git a/app/src/main/java/com/keylesspalace/tusky/components/account/list/ListSelectionFragment.kt b/app/src/main/java/com/keylesspalace/tusky/components/account/list/ListSelectionFragment.kt index 7a1cb6c87..9d1b4e4b7 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/account/list/ListSelectionFragment.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/account/list/ListSelectionFragment.kt @@ -56,13 +56,6 @@ class ListSelectionFragment : DialogFragment() { private val viewModel: ListsForAccountViewModel by viewModels() - private var _binding: FragmentListsListBinding? = null - - // This property is only valid between onCreateDialog and onDestroyView - private val binding get() = _binding!! - - private val adapter = Adapter() - private var selectListener: ListSelectionListener? = null private var accountId: String? = null @@ -80,7 +73,8 @@ class ListSelectionFragment : DialogFragment() { override fun onCreateDialog(savedInstanceState: Bundle?): Dialog { val context = requireContext() - _binding = FragmentListsListBinding.inflate(layoutInflater) + val binding = FragmentListsListBinding.inflate(layoutInflater) + val adapter = Adapter() binding.listsView.adapter = adapter val dialogBuilder = AlertDialog.Builder(context) @@ -120,7 +114,7 @@ class ListSelectionFragment : DialogFragment() { binding.listsView.hide() binding.messageView.apply { show() - setup(error) { load() } + setup(error) { load(binding) } } } } @@ -155,7 +149,7 @@ class ListSelectionFragment : DialogFragment() { } lifecycleScope.launch { - load() + load(binding) } return dialog @@ -173,12 +167,7 @@ class ListSelectionFragment : DialogFragment() { } } - override fun onDestroyView() { - super.onDestroyView() - _binding = null - } - - private fun load() { + private fun load(binding: FragmentListsListBinding) { binding.progressBar.show() binding.listsView.hide() binding.messageView.hide() diff --git a/app/src/main/java/com/keylesspalace/tusky/components/account/media/AccountMediaFragment.kt b/app/src/main/java/com/keylesspalace/tusky/components/account/media/AccountMediaFragment.kt index 1847e0481..0eb61018c 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/account/media/AccountMediaFragment.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/account/media/AccountMediaFragment.kt @@ -71,7 +71,7 @@ class AccountMediaFragment : private val viewModel: AccountMediaViewModel by viewModels() - private lateinit var adapter: AccountMediaGridAdapter + private var adapter: AccountMediaGridAdapter? = null override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) @@ -83,11 +83,12 @@ class AccountMediaFragment : val useBlurhash = preferences.getBoolean(PrefKeys.USE_BLURHASH, true) - adapter = AccountMediaGridAdapter( + val adapter = AccountMediaGridAdapter( useBlurhash = useBlurhash, context = view.context, onAttachmentClickListener = ::onAttachmentClick ) + this.adapter = adapter val columnCount = view.context.resources.getInteger(R.integer.profile_media_column_count) val imageSpacing = view.context.resources.getDimensionPixelSize( @@ -145,6 +146,12 @@ class AccountMediaFragment : } } + override fun onDestroyView() { + // Clear the adapter to prevent leaking the View + adapter = null + super.onDestroyView() + } + override fun onCreateMenu(menu: Menu, menuInflater: MenuInflater) { menuInflater.inflate(R.menu.fragment_account_media, menu) menu.findItem(R.id.action_refresh)?.apply { @@ -206,7 +213,7 @@ class AccountMediaFragment : } override fun refreshContent() { - adapter.refresh() + adapter?.refresh() } companion object { diff --git a/app/src/main/java/com/keylesspalace/tusky/components/accountlist/AccountListFragment.kt b/app/src/main/java/com/keylesspalace/tusky/components/accountlist/AccountListFragment.kt index c48d1f3d3..559b9b11b 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/accountlist/AccountListFragment.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/accountlist/AccountListFragment.kt @@ -81,8 +81,7 @@ class AccountListFragment : private lateinit var type: Type private var id: String? = null - private lateinit var scrollListener: EndlessOnScrollListener - private lateinit var adapter: AccountAdapter<*> + private var adapter: AccountAdapter<*>? = null private var fetching = false private var bottomId: String? = null @@ -101,16 +100,13 @@ class AccountListFragment : DividerItemDecoration(view.context, DividerItemDecoration.VERTICAL) ) - binding.swipeRefreshLayout.setOnRefreshListener { fetchAccounts() } - binding.swipeRefreshLayout.setColorSchemeResources(R.color.tusky_blue) - val animateAvatar = preferences.getBoolean(PrefKeys.ANIMATE_GIF_AVATARS, false) val animateEmojis = preferences.getBoolean(PrefKeys.ANIMATE_CUSTOM_EMOJIS, false) val showBotOverlay = preferences.getBoolean(PrefKeys.SHOW_BOT_OVERLAY, true) val activeAccount = accountManager.activeAccount!! - adapter = when (type) { + val adapter = when (type) { Type.BLOCKS -> BlocksAdapter(this, animateAvatar, animateEmojis, showBotOverlay) Type.MUTES -> MutesAdapter(this, animateAvatar, animateEmojis, showBotOverlay) Type.FOLLOW_REQUESTS -> { @@ -125,22 +121,32 @@ class AccountListFragment : } else -> FollowAdapter(this, animateAvatar, animateEmojis, showBotOverlay) } + this.adapter = adapter if (binding.recyclerView.adapter == null) { binding.recyclerView.adapter = adapter } - scrollListener = object : EndlessOnScrollListener(layoutManager) { + val scrollListener = object : EndlessOnScrollListener(layoutManager) { override fun onLoadMore(totalItemsCount: Int, view: RecyclerView) { if (bottomId == null) { return } - fetchAccounts(bottomId) + fetchAccounts(adapter, bottomId) } } binding.recyclerView.addOnScrollListener(scrollListener) - fetchAccounts() + binding.swipeRefreshLayout.setOnRefreshListener { fetchAccounts(adapter) } + binding.swipeRefreshLayout.setColorSchemeResources(R.color.tusky_blue) + + fetchAccounts(adapter) + } + + override fun onDestroyView() { + // Clear the adapter to prevent leaking the View + adapter = null + super.onDestroyView() } override fun onViewTag(tag: String) { @@ -303,7 +309,7 @@ class AccountListFragment : return requireNotNull(id) { "id must not be null for type " + type.name } } - private fun fetchAccounts(fromId: String? = null) { + private fun fetchAccounts(adapter: AccountAdapter<*>, fromId: String? = null) { if (fetching) { return } @@ -319,19 +325,19 @@ class AccountListFragment : val response = getFetchCallByListType(fromId) if (!response.isSuccessful) { - onFetchAccountsFailure(Exception(response.message())) + onFetchAccountsFailure(adapter, Exception(response.message())) return@launch } val accountList = response.body() if (accountList == null) { - onFetchAccountsFailure(Exception(response.message())) + onFetchAccountsFailure(adapter, Exception(response.message())) return@launch } val linkHeader = response.headers()["Link"] - onFetchAccountsSuccess(accountList, linkHeader) + onFetchAccountsSuccess(adapter, accountList, linkHeader) } catch (exception: Exception) { if (exception is CancellationException) { // Scope is cancelled, probably because the fragment is destroyed. @@ -339,12 +345,16 @@ class AccountListFragment : // (CancellationException in a cancelled scope is normal and will be ignored) throw exception } - onFetchAccountsFailure(exception) + onFetchAccountsFailure(adapter, exception) } } } - private fun onFetchAccountsSuccess(accounts: List, linkHeader: String?) { + private fun onFetchAccountsSuccess( + adapter: AccountAdapter<*>, + accounts: List, + linkHeader: String? + ) { adapter.setBottomLoading(false) binding.swipeRefreshLayout.isRefreshing = false @@ -359,7 +369,7 @@ class AccountListFragment : } if (adapter is MutesAdapter) { - fetchRelationships(accounts.map { it.id }) + fetchRelationships(adapter, accounts.map { it.id }) } bottomId = fromId @@ -378,23 +388,30 @@ class AccountListFragment : } } - private fun fetchRelationships(ids: List) { - lifecycleScope.launch { + private fun fetchRelationships(mutesAdapter: MutesAdapter, ids: List) { + viewLifecycleOwner.lifecycleScope.launch { api.relationships(ids) - .fold(::onFetchRelationshipsSuccess) { throwable -> - Log.e(TAG, "Fetch failure for relationships of accounts: $ids", throwable) - } + .fold( + onSuccess = { relationships -> + onFetchRelationshipsSuccess(mutesAdapter, relationships) + }, + onFailure = { throwable -> + Log.e(TAG, "Fetch failure for relationships of accounts: $ids", throwable) + } + ) } } - private fun onFetchRelationshipsSuccess(relationships: List) { - val mutesAdapter = adapter as MutesAdapter + private fun onFetchRelationshipsSuccess( + mutesAdapter: MutesAdapter, + relationships: List + ) { val mutingNotificationsMap = HashMap() relationships.map { mutingNotificationsMap.put(it.id, it.mutingNotifications) } mutesAdapter.updateMutingNotificationsMap(mutingNotificationsMap) } - private fun onFetchAccountsFailure(throwable: Throwable) { + private fun onFetchAccountsFailure(adapter: AccountAdapter<*>, throwable: Throwable) { fetching = false binding.swipeRefreshLayout.isRefreshing = false Log.e(TAG, "Fetch failure", throwable) @@ -403,7 +420,7 @@ class AccountListFragment : binding.messageView.show() binding.messageView.setup(throwable) { binding.messageView.hide() - this.fetchAccounts(null) + this.fetchAccounts(adapter, null) } } } diff --git a/app/src/main/java/com/keylesspalace/tusky/components/conversation/ConversationsFragment.kt b/app/src/main/java/com/keylesspalace/tusky/components/conversation/ConversationsFragment.kt index 3d15093f7..36db437c3 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/conversation/ConversationsFragment.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/conversation/ConversationsFragment.kt @@ -17,12 +17,10 @@ package com.keylesspalace.tusky.components.conversation import android.content.SharedPreferences import android.os.Bundle -import android.view.LayoutInflater import android.view.Menu import android.view.MenuInflater import android.view.MenuItem import android.view.View -import android.view.ViewGroup import androidx.appcompat.app.AlertDialog import androidx.appcompat.widget.PopupMenu import androidx.core.view.MenuProvider @@ -71,7 +69,7 @@ import kotlinx.coroutines.launch @AndroidEntryPoint class ConversationsFragment : - SFragment(), + SFragment(R.layout.fragment_timeline), StatusActionListener, ReselectableFragment, MenuProvider { @@ -86,18 +84,10 @@ class ConversationsFragment : private val binding by viewBinding(FragmentTimelineBinding::bind) - private lateinit var adapter: ConversationAdapter + private var adapter: ConversationAdapter? = null private var hideFab = false - override fun onCreateView( - inflater: LayoutInflater, - container: ViewGroup?, - savedInstanceState: Bundle? - ): View? { - return inflater.inflate(R.layout.fragment_timeline, container, false) - } - override fun onViewCreated(view: View, savedInstanceState: Bundle?) { requireActivity().addMenuProvider(this, viewLifecycleOwner, Lifecycle.State.RESUMED) @@ -117,9 +107,10 @@ class ConversationsFragment : openSpoiler = accountManager.activeAccount!!.alwaysOpenSpoiler ) - adapter = ConversationAdapter(statusDisplayOptions, this) + val adapter = ConversationAdapter(statusDisplayOptions, this) + this.adapter = adapter - setupRecyclerView() + setupRecyclerView(adapter) initSwipeToRefresh() @@ -132,7 +123,7 @@ class ConversationsFragment : binding.progressBar.hide() if (loadState.isAnyLoading()) { - lifecycleScope.launch { + viewLifecycleOwner.lifecycleScope.launch { eventHub.dispatch( ConversationsLoadingEvent( accountManager.activeAccount?.accountId ?: "" @@ -219,15 +210,21 @@ class ConversationsFragment : } } - lifecycleScope.launch { + viewLifecycleOwner.lifecycleScope.launch { eventHub.events.collect { event -> if (event is PreferenceChangedEvent) { - onPreferenceChanged(event.preferenceKey) + onPreferenceChanged(adapter, event.preferenceKey) } } } } + override fun onDestroyView() { + // Clear the adapter to prevent leaking the View + adapter = null + super.onDestroyView() + } + override fun onCreateMenu(menu: Menu, menuInflater: MenuInflater) { menuInflater.inflate(R.menu.fragment_conversations, menu) menu.findItem(R.id.action_refresh)?.apply { @@ -250,7 +247,7 @@ class ConversationsFragment : } } - private fun setupRecyclerView() { + private fun setupRecyclerView(adapter: ConversationAdapter) { binding.recyclerView.setHasFixedSize(true) binding.recyclerView.layoutManager = LinearLayoutManager(context) @@ -265,7 +262,7 @@ class ConversationsFragment : } private fun refreshContent() { - adapter.refresh() + adapter?.refresh() } private fun initSwipeToRefresh() { @@ -278,13 +275,13 @@ class ConversationsFragment : } override fun onFavourite(favourite: Boolean, position: Int) { - adapter.peek(position)?.let { conversation -> + adapter?.peek(position)?.let { conversation -> viewModel.favourite(favourite, conversation) } } override fun onBookmark(favourite: Boolean, position: Int) { - adapter.peek(position)?.let { conversation -> + adapter?.peek(position)?.let { conversation -> viewModel.bookmark(favourite, conversation) } } @@ -292,7 +289,7 @@ class ConversationsFragment : override val onMoreTranslate: ((translate: Boolean, position: Int) -> Unit)? = null override fun onMore(view: View, position: Int) { - adapter.peek(position)?.let { conversation -> + adapter?.peek(position)?.let { conversation -> val popup = PopupMenu(requireContext(), view) popup.inflate(R.menu.conversation_more) @@ -316,7 +313,7 @@ class ConversationsFragment : } override fun onViewMedia(position: Int, attachmentIndex: Int, view: View?) { - adapter.peek(position)?.let { conversation -> + adapter?.peek(position)?.let { conversation -> viewMedia( attachmentIndex, AttachmentViewData.list(conversation.lastStatus), @@ -326,7 +323,7 @@ class ConversationsFragment : } override fun onViewThread(position: Int) { - adapter.peek(position)?.let { conversation -> + adapter?.peek(position)?.let { conversation -> viewThread(conversation.lastStatus.id, conversation.lastStatus.status.url) } } @@ -336,13 +333,13 @@ class ConversationsFragment : } override fun onExpandedChange(expanded: Boolean, position: Int) { - adapter.peek(position)?.let { conversation -> + adapter?.peek(position)?.let { conversation -> viewModel.expandHiddenStatus(expanded, conversation) } } override fun onContentHiddenChange(isShowing: Boolean, position: Int) { - adapter.peek(position)?.let { conversation -> + adapter?.peek(position)?.let { conversation -> viewModel.showContent(isShowing, conversation) } } @@ -352,7 +349,7 @@ class ConversationsFragment : } override fun onContentCollapsedChange(isCollapsed: Boolean, position: Int) { - adapter.peek(position)?.let { conversation -> + adapter?.peek(position)?.let { conversation -> viewModel.collapseLongStatus(isCollapsed, conversation) } } @@ -372,13 +369,13 @@ class ConversationsFragment : } override fun onReply(position: Int) { - adapter.peek(position)?.let { conversation -> + adapter?.peek(position)?.let { conversation -> reply(conversation.lastStatus.status) } } override fun onVoteInPoll(position: Int, choices: MutableList) { - adapter.peek(position)?.let { conversation -> + adapter?.peek(position)?.let { conversation -> viewModel.voteInPoll(choices, conversation) } } @@ -387,7 +384,7 @@ class ConversationsFragment : } override fun onReselect() { - if (isAdded) { + if (view != null) { binding.recyclerView.layoutManager?.scrollToPosition(0) binding.recyclerView.stopScroll() } @@ -407,7 +404,7 @@ class ConversationsFragment : .show() } - private fun onPreferenceChanged(key: String) { + private fun onPreferenceChanged(adapter: ConversationAdapter, key: String) { when (key) { PrefKeys.FAB_HIDE -> { hideFab = preferences.getBoolean(PrefKeys.FAB_HIDE, false) diff --git a/app/src/main/java/com/keylesspalace/tusky/components/domainblocks/DomainBlocksFragment.kt b/app/src/main/java/com/keylesspalace/tusky/components/domainblocks/DomainBlocksFragment.kt index 5cadce707..10438087e 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/domainblocks/DomainBlocksFragment.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/domainblocks/DomainBlocksFragment.kt @@ -43,7 +43,7 @@ class DomainBlocksFragment : Fragment(R.layout.fragment_domain_blocks) { } } - lifecycleScope.launch { + viewLifecycleOwner.lifecycleScope.launch { viewModel.domainPager.collectLatest { pagingData -> adapter.submitData(pagingData) } diff --git a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsFragment.kt b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsFragment.kt index f9f01c49e..33e51b6ef 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsFragment.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsFragment.kt @@ -80,7 +80,7 @@ import kotlinx.coroutines.launch @AndroidEntryPoint class NotificationsFragment : - SFragment(), + SFragment(R.layout.fragment_timeline_notifications), SwipeRefreshLayout.OnRefreshListener, StatusActionListener, NotificationActionListener, @@ -98,7 +98,7 @@ class NotificationsFragment : private val viewModel: NotificationsViewModel by viewModels() - private lateinit var adapter: NotificationsPagingAdapter + private var adapter: NotificationsPagingAdapter? = null private var hideFab: Boolean = false private var showNotificationsFilterBar: Boolean = true @@ -108,14 +108,6 @@ class NotificationsFragment : private var loadMorePosition: Int? = null private var statusIdBelowLoadMore: String? = null - override fun onCreateView( - inflater: LayoutInflater, - container: ViewGroup?, - savedInstanceState: Bundle? - ): View? { - return inflater.inflate(R.layout.fragment_timeline_notifications, container, false) - } - override fun onViewCreated(view: View, savedInstanceState: Bundle?) { requireActivity().addMenuProvider(this, viewLifecycleOwner, Lifecycle.State.RESUMED) @@ -151,13 +143,14 @@ class NotificationsFragment : // Setup the RecyclerView. binding.recyclerView.setHasFixedSize(true) - adapter = NotificationsPagingAdapter( + val adapter = NotificationsPagingAdapter( accountId = accountManager.activeAccount!!.accountId, statusListener = this, notificationActionListener = this, accountActionListener = this, statusDisplayOptions = statusDisplayOptions ) + this.adapter = adapter binding.recyclerView.layoutManager = LinearLayoutManager(context) binding.recyclerView.setAccessibilityDelegateCompat( ListStatusAccessibilityDelegate( @@ -247,7 +240,7 @@ class NotificationsFragment : } } if (readingOrder == ReadingOrder.OLDEST_FIRST) { - updateReadingPositionForOldestFirst() + updateReadingPositionForOldestFirst(adapter) } } }) @@ -261,7 +254,7 @@ class NotificationsFragment : viewLifecycleOwner.lifecycleScope.launch { eventHub.events.collect { event -> if (event is PreferenceChangedEvent) { - onPreferenceChanged(event.preferenceKey) + onPreferenceChanged(adapter, event.preferenceKey) } } } @@ -285,15 +278,21 @@ class NotificationsFragment : } } + override fun onDestroyView() { + // Clear the adapter to prevent leaking the View + adapter = null + super.onDestroyView() + } + override fun onReselect() { - if (isAdded) { + if (view != null) { binding.recyclerView.layoutManager?.scrollToPosition(0) binding.recyclerView.stopScroll() } } override fun onRefresh() { - adapter.refresh() + adapter?.refresh() } override fun onViewAccount(id: String) { @@ -309,7 +308,7 @@ class NotificationsFragment : } override fun onRespondToFollowRequest(accept: Boolean, id: String, position: Int) { - val notification = adapter.peek(position) ?: return + val notification = adapter?.peek(position) ?: return viewModel.respondToFollowRequest(accept, accountId = id, notificationId = notification.id) } @@ -324,17 +323,17 @@ class NotificationsFragment : } override fun onReply(position: Int) { - val status = adapter.peek(position)?.asStatusOrNull() ?: return + val status = adapter?.peek(position)?.asStatusOrNull() ?: return super.reply(status.status) } override fun removeItem(position: Int) { - val notification = adapter.peek(position) ?: return + val notification = adapter?.peek(position) ?: return viewModel.remove(notification.id) } override fun onReblog(reblog: Boolean, position: Int) { - val status = adapter.peek(position)?.asStatusOrNull() ?: return + val status = adapter?.peek(position)?.asStatusOrNull() ?: return viewModel.reblog(reblog, status) } @@ -348,8 +347,8 @@ class NotificationsFragment : } private fun onTranslate(position: Int) { - val status = adapter.peek(position)?.asStatusOrNull() ?: return - lifecycleScope.launch { + val status = adapter?.peek(position)?.asStatusOrNull() ?: return + viewLifecycleOwner.lifecycleScope.launch { viewModel.translate(status) .onFailure { Snackbar.make( @@ -362,32 +361,32 @@ class NotificationsFragment : } override fun onUntranslate(position: Int) { - val status = adapter.peek(position)?.asStatusOrNull() ?: return + val status = adapter?.peek(position)?.asStatusOrNull() ?: return viewModel.untranslate(status) } override fun onFavourite(favourite: Boolean, position: Int) { - val status = adapter.peek(position)?.asStatusOrNull() ?: return + val status = adapter?.peek(position)?.asStatusOrNull() ?: return viewModel.favorite(favourite, status) } override fun onBookmark(bookmark: Boolean, position: Int) { - val status = adapter.peek(position)?.asStatusOrNull() ?: return + val status = adapter?.peek(position)?.asStatusOrNull() ?: return viewModel.bookmark(bookmark, status) } override fun onVoteInPoll(position: Int, choices: List) { - val status = adapter.peek(position)?.asStatusOrNull() ?: return + val status = adapter?.peek(position)?.asStatusOrNull() ?: return viewModel.voteInPoll(choices, status) } override fun clearWarningAction(position: Int) { - val status = adapter.peek(position)?.asStatusOrNull() ?: return + val status = adapter?.peek(position)?.asStatusOrNull() ?: return viewModel.clearWarning(status) } override fun onMore(view: View, position: Int) { - val status = adapter.peek(position)?.asStatusOrNull() ?: return + val status = adapter?.peek(position)?.asStatusOrNull() ?: return super.more( status.status, view, @@ -397,32 +396,33 @@ class NotificationsFragment : } override fun onViewMedia(position: Int, attachmentIndex: Int, view: View?) { - val status = adapter.peek(position)?.asStatusOrNull() ?: return + val status = adapter?.peek(position)?.asStatusOrNull() ?: return super.viewMedia(attachmentIndex, AttachmentViewData.list(status), view) } override fun onViewThread(position: Int) { - val status = adapter.peek(position)?.asStatusOrNull()?.status ?: return + val status = adapter?.peek(position)?.asStatusOrNull()?.status ?: return super.viewThread(status.id, status.url) } override fun onOpenReblog(position: Int) { - val status = adapter.peek(position)?.asStatusOrNull() ?: return + val status = adapter?.peek(position)?.asStatusOrNull() ?: return super.openReblog(status.status) } override fun onExpandedChange(expanded: Boolean, position: Int) { - val status = adapter.peek(position)?.asStatusOrNull() ?: return + val status = adapter?.peek(position)?.asStatusOrNull() ?: return viewModel.changeExpanded(expanded, status) } override fun onContentHiddenChange(isShowing: Boolean, position: Int) { - val status = adapter.peek(position)?.asStatusOrNull() ?: return + val status = adapter?.peek(position)?.asStatusOrNull() ?: return viewModel.changeContentShowing(isShowing, status) } override fun onLoadMore(position: Int) { - val placeholder = adapter.peek(position)?.asPlaceholderOrNull() ?: return + val adapter = this.adapter + val placeholder = adapter?.peek(position)?.asPlaceholderOrNull() ?: return loadMorePosition = position statusIdBelowLoadMore = if (position + 1 < adapter.itemCount) adapter.peek(position + 1)?.id else null @@ -430,7 +430,7 @@ class NotificationsFragment : } override fun onContentCollapsedChange(isCollapsed: Boolean, position: Int) { - val status = adapter.peek(position)?.asStatusOrNull() ?: return + val status = adapter?.peek(position)?.asStatusOrNull() ?: return viewModel.changeContentCollapsed(isCollapsed, status) } @@ -478,7 +478,7 @@ class NotificationsFragment : window.showAsDropDown(binding.buttonFilter) } - private fun onPreferenceChanged(key: String) { + private fun onPreferenceChanged(adapter: NotificationsPagingAdapter, key: String) { when (key) { PrefKeys.FAB_HIDE -> { hideFab = preferences.getBoolean(PrefKeys.FAB_HIDE, false) @@ -493,7 +493,7 @@ class NotificationsFragment : } PrefKeys.SHOW_NOTIFICATIONS_FILTER -> { - if (isAdded) { + if (view != null) { showNotificationsFilterBar = preferences.getBoolean(PrefKeys.SHOW_NOTIFICATIONS_FILTER, true) updateFilterBarVisibility() } @@ -522,7 +522,7 @@ class NotificationsFragment : } } - private fun updateReadingPositionForOldestFirst() { + private fun updateReadingPositionForOldestFirst(adapter: NotificationsPagingAdapter) { var position = loadMorePosition ?: return val notificationIdBelowLoadMore = statusIdBelowLoadMore ?: return diff --git a/app/src/main/java/com/keylesspalace/tusky/components/report/fragments/ReportStatusesFragment.kt b/app/src/main/java/com/keylesspalace/tusky/components/report/fragments/ReportStatusesFragment.kt index ed3139c66..f52cef46f 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/report/fragments/ReportStatusesFragment.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/report/fragments/ReportStatusesFragment.kt @@ -80,7 +80,7 @@ class ReportStatusesFragment : private val binding by viewBinding(FragmentReportStatusesBinding::bind) - private lateinit var adapter: StatusesAdapter + private var adapter: StatusesAdapter? = null private var snackbarErrorRetry: Snackbar? = null @@ -114,6 +114,13 @@ class ReportStatusesFragment : setupSwipeRefreshLayout() } + override fun onDestroyView() { + // Clear the adapter to prevent leaking the View + adapter = null + snackbarErrorRetry = null + super.onDestroyView() + } + override fun onCreateMenu(menu: Menu, menuInflater: MenuInflater) { menuInflater.inflate(R.menu.fragment_report_statuses, menu) menu.findItem(R.id.action_refresh)?.apply { @@ -137,7 +144,8 @@ class ReportStatusesFragment : override fun onRefresh() { snackbarErrorRetry?.dismiss() - adapter.refresh() + snackbarErrorRetry = null + adapter?.refresh() } private fun setupSwipeRefreshLayout() { @@ -163,7 +171,8 @@ class ReportStatusesFragment : openSpoiler = accountManager.activeAccount!!.alwaysOpenSpoiler ) - adapter = StatusesAdapter(statusDisplayOptions, viewModel.statusViewState, this) + val adapter = StatusesAdapter(statusDisplayOptions, viewModel.statusViewState, this) + this.adapter = adapter binding.recyclerView.addItemDecoration( DividerItemDecoration(requireContext(), DividerItemDecoration.VERTICAL) @@ -172,7 +181,7 @@ class ReportStatusesFragment : binding.recyclerView.adapter = adapter (binding.recyclerView.itemAnimator as SimpleItemAnimator).supportsChangeAnimations = false - lifecycleScope.launch { + viewLifecycleOwner.lifecycleScope.launch { viewModel.statusesFlow.collectLatest { pagingData -> adapter.submitData(pagingData) } @@ -183,7 +192,7 @@ class ReportStatusesFragment : loadState.append is LoadState.Error || loadState.prepend is LoadState.Error ) { - showError() + showError(adapter) } binding.progressBarBottom.visible(loadState.append == LoadState.Loading) @@ -198,13 +207,15 @@ class ReportStatusesFragment : } } - private fun showError() { + private fun showError(adapter: StatusesAdapter) { if (snackbarErrorRetry?.isShown != true) { - snackbarErrorRetry = Snackbar.make(binding.swipeRefreshLayout, R.string.failed_fetch_posts, Snackbar.LENGTH_INDEFINITE) - snackbarErrorRetry?.setAction(R.string.action_retry) { - adapter.retry() - } - snackbarErrorRetry?.show() + snackbarErrorRetry = + Snackbar.make(binding.swipeRefreshLayout, R.string.failed_fetch_posts, Snackbar.LENGTH_INDEFINITE) + .setAction(R.string.action_retry) { + adapter.retry() + }.also { + it.show() + } } } diff --git a/app/src/main/java/com/keylesspalace/tusky/components/search/fragments/SearchFragment.kt b/app/src/main/java/com/keylesspalace/tusky/components/search/fragments/SearchFragment.kt index 526e47cc8..31e673c3a 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/search/fragments/SearchFragment.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/search/fragments/SearchFragment.kt @@ -56,15 +56,22 @@ abstract class SearchFragment : abstract fun createAdapter(): PagingDataAdapter abstract val data: Flow> - protected lateinit var adapter: PagingDataAdapter + protected var adapter: PagingDataAdapter? = null private var currentQuery: String = "" override fun onViewCreated(view: View, savedInstanceState: Bundle?) { - initAdapter() + val adapter = initAdapter() setupSwipeRefreshLayout() requireActivity().addMenuProvider(this, viewLifecycleOwner, Lifecycle.State.RESUMED) - subscribeObservables() + subscribeObservables(adapter) + } + + override fun onDestroyView() { + // Clear the adapter to prevent leaking the View + adapter = null + snackbarErrorRetry = null + super.onDestroyView() } private fun setupSwipeRefreshLayout() { @@ -72,7 +79,7 @@ abstract class SearchFragment : binding.swipeRefreshLayout.setColorSchemeResources(R.color.tusky_blue) } - private fun subscribeObservables() { + private fun subscribeObservables(adapter: PagingDataAdapter) { viewLifecycleOwner.lifecycleScope.launch { data.collectLatest { pagingData -> adapter.submitData(pagingData) @@ -82,7 +89,7 @@ abstract class SearchFragment : adapter.addLoadStateListener { loadState -> if (loadState.refresh is LoadState.Error) { - showError() + showError(adapter) } val isNewSearch = currentQuery != viewModel.currentQuery @@ -128,22 +135,26 @@ abstract class SearchFragment : } } - private fun initAdapter() { + private fun initAdapter(): PagingDataAdapter { binding.searchRecyclerView.layoutManager = LinearLayoutManager(binding.searchRecyclerView.context) - adapter = createAdapter() + val adapter = createAdapter() + this.adapter = adapter binding.searchRecyclerView.adapter = adapter binding.searchRecyclerView.setHasFixedSize(true) (binding.searchRecyclerView.itemAnimator as SimpleItemAnimator).supportsChangeAnimations = false + return adapter } - private fun showError() { + private fun showError(adapter: PagingDataAdapter) { if (snackbarErrorRetry?.isShown != true) { - snackbarErrorRetry = Snackbar.make(binding.root, R.string.failed_search, Snackbar.LENGTH_INDEFINITE) - snackbarErrorRetry?.setAction(R.string.action_retry) { - snackbarErrorRetry = null - adapter.retry() - } - snackbarErrorRetry?.show() + snackbarErrorRetry = + Snackbar.make(binding.root, R.string.failed_search, Snackbar.LENGTH_INDEFINITE) + .setAction(R.string.action_retry) { + snackbarErrorRetry = null + adapter.retry() + }.also { + it.show() + } } } @@ -167,6 +178,8 @@ abstract class SearchFragment : get() = (activity as? BottomSheetActivity) override fun onRefresh() { - adapter.refresh() + snackbarErrorRetry?.dismiss() + snackbarErrorRetry = null + adapter?.refresh() } } diff --git a/app/src/main/java/com/keylesspalace/tusky/components/search/fragments/SearchStatusesFragment.kt b/app/src/main/java/com/keylesspalace/tusky/components/search/fragments/SearchStatusesFragment.kt index d13e05457..8bad5e629 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/search/fragments/SearchStatusesFragment.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/search/fragments/SearchStatusesFragment.kt @@ -82,9 +82,6 @@ class SearchStatusesFragment : SearchFragment(), Status override val data: Flow> get() = viewModel.statusesFlow - private val searchAdapter - get() = super.adapter as SearchStatusesAdapter - private var pendingMediaDownloads: List? = null private val downloadAllMediaPermissionLauncher = @@ -129,6 +126,7 @@ class SearchStatusesFragment : SearchFragment(), Status showSensitiveMedia = accountManager.activeAccount!!.alwaysShowSensitiveMedia, openSpoiler = accountManager.activeAccount!!.alwaysOpenSpoiler ) + val adapter = SearchStatusesAdapter(statusDisplayOptions, this) binding.searchRecyclerView.setAccessibilityDelegateCompat( ListStatusAccessibilityDelegate(binding.searchRecyclerView, this) { pos -> @@ -148,41 +146,41 @@ class SearchStatusesFragment : SearchFragment(), Status ) binding.searchRecyclerView.layoutManager = LinearLayoutManager(binding.searchRecyclerView.context) - return SearchStatusesAdapter(statusDisplayOptions, this) + return adapter } override fun onContentHiddenChange(isShowing: Boolean, position: Int) { - searchAdapter.peek(position)?.let { + adapter?.peek(position)?.let { viewModel.contentHiddenChange(it, isShowing) } } override fun onReply(position: Int) { - searchAdapter.peek(position)?.let { status -> + adapter?.peek(position)?.let { status -> reply(status) } } override fun onFavourite(favourite: Boolean, position: Int) { - searchAdapter.peek(position)?.let { status -> + adapter?.peek(position)?.let { status -> viewModel.favorite(status, favourite) } } override fun onBookmark(bookmark: Boolean, position: Int) { - searchAdapter.peek(position)?.let { status -> + adapter?.peek(position)?.let { status -> viewModel.bookmark(status, bookmark) } } override fun onMore(view: View, position: Int) { - searchAdapter.peek(position)?.let { + adapter?.peek(position)?.let { more(it, view, position) } } override fun onViewMedia(position: Int, attachmentIndex: Int, view: View?) { - searchAdapter.peek(position)?.let { status -> + adapter?.peek(position)?.let { status -> when (status.attachments[attachmentIndex].type) { Attachment.Type.GIFV, Attachment.Type.VIDEO, Attachment.Type.IMAGE, Attachment.Type.AUDIO -> { val attachments = AttachmentViewData.list(status) @@ -213,20 +211,20 @@ class SearchStatusesFragment : SearchFragment(), Status } override fun onViewThread(position: Int) { - searchAdapter.peek(position)?.status?.let { status -> + adapter?.peek(position)?.status?.let { status -> val actionableStatus = status.actionableStatus bottomSheetActivity?.viewThread(actionableStatus.id, actionableStatus.url) } } override fun onOpenReblog(position: Int) { - searchAdapter.peek(position)?.status?.let { status -> + adapter?.peek(position)?.status?.let { status -> bottomSheetActivity?.viewAccount(status.account.id) } } override fun onExpandedChange(expanded: Boolean, position: Int) { - searchAdapter.peek(position)?.let { + adapter?.peek(position)?.let { viewModel.expandedChange(it, expanded) } } @@ -236,13 +234,13 @@ class SearchStatusesFragment : SearchFragment(), Status } override fun onContentCollapsedChange(isCollapsed: Boolean, position: Int) { - searchAdapter.peek(position)?.let { + adapter?.peek(position)?.let { viewModel.collapsedChange(it, isCollapsed) } } override fun onVoteInPoll(position: Int, choices: MutableList) { - searchAdapter.peek(position)?.let { + adapter?.peek(position)?.let { viewModel.voteInPoll(it, choices) } } @@ -250,19 +248,19 @@ class SearchStatusesFragment : SearchFragment(), Status override fun clearWarningAction(position: Int) {} private fun removeItem(position: Int) { - searchAdapter.peek(position)?.let { + adapter?.peek(position)?.let { viewModel.removeItem(it) } } override fun onReblog(reblog: Boolean, position: Int) { - searchAdapter.peek(position)?.let { status -> + adapter?.peek(position)?.let { status -> viewModel.reblog(status, reblog) } } override fun onUntranslate(position: Int) { - searchAdapter.peek(position)?.let { + adapter?.peek(position)?.let { viewModel.untranslate(it) } } @@ -418,7 +416,7 @@ class SearchStatusesFragment : SearchFragment(), Status } R.id.status_mute_conversation -> { - searchAdapter.peek(position)?.let { foundStatus -> + adapter?.peek(position)?.let { foundStatus -> viewModel.muteConversation(foundStatus, !status.muted) } return@setOnMenuItemClickListener true @@ -460,7 +458,7 @@ class SearchStatusesFragment : SearchFragment(), Status } R.id.status_edit -> { - editStatus(id, position, status) + editStatus(id, status) return@setOnMenuItemClickListener true } @@ -473,7 +471,7 @@ class SearchStatusesFragment : SearchFragment(), Status if (statusViewData.translation != null) { viewModel.untranslate(statusViewData) } else { - lifecycleScope.launch { + viewLifecycleOwner.lifecycleScope.launch { viewModel.translate(statusViewData) .onFailure { Snackbar.make( @@ -574,11 +572,11 @@ class SearchStatusesFragment : SearchFragment(), Status } private fun showConfirmEditDialog(id: String, position: Int, status: Status) { - activity?.let { - AlertDialog.Builder(it) + context?.let { context -> + AlertDialog.Builder(context) .setMessage(R.string.dialog_redraft_post_warning) .setPositiveButton(android.R.string.ok) { _, _ -> - lifecycleScope.launch { + viewLifecycleOwner.lifecycleScope.launch { viewModel.deleteStatusAsync(id).await().fold( { deletedStatus -> removeItem(position) @@ -590,7 +588,7 @@ class SearchStatusesFragment : SearchFragment(), Status } val intent = ComposeActivity.startIntent( - requireContext(), + context, ComposeOptions( content = redraftStatus.text.orEmpty(), inReplyToId = redraftStatus.inReplyToId, @@ -621,8 +619,8 @@ class SearchStatusesFragment : SearchFragment(), Status } } - private fun editStatus(id: String, position: Int, status: Status) { - lifecycleScope.launch { + private fun editStatus(id: String, status: Status) { + viewLifecycleOwner.lifecycleScope.launch { mastodonApi.statusSource(id).fold( { source -> val composeOptions = ComposeOptions( diff --git a/app/src/main/java/com/keylesspalace/tusky/components/timeline/TimelineFragment.kt b/app/src/main/java/com/keylesspalace/tusky/components/timeline/TimelineFragment.kt index 135cb0635..6588cfce2 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/timeline/TimelineFragment.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/timeline/TimelineFragment.kt @@ -18,14 +18,12 @@ package com.keylesspalace.tusky.components.timeline import android.content.SharedPreferences import android.os.Bundle import android.util.Log -import android.view.LayoutInflater import android.view.Menu import android.view.MenuInflater import android.view.MenuItem import android.view.View -import android.view.ViewGroup import android.view.accessibility.AccessibilityManager -import androidx.core.content.ContextCompat +import androidx.core.content.getSystemService import androidx.core.view.MenuProvider import androidx.lifecycle.Lifecycle import androidx.lifecycle.ViewModelProvider @@ -82,7 +80,7 @@ import kotlinx.coroutines.launch @AndroidEntryPoint class TimelineFragment : - SFragment(), + SFragment(R.layout.fragment_timeline), OnRefreshListener, StatusActionListener, ReselectableFragment, @@ -108,7 +106,7 @@ class TimelineFragment : private lateinit var kind: TimelineViewModel.Kind - private lateinit var adapter: TimelinePagingAdapter + private var adapter: TimelinePagingAdapter? = null private var isSwipeToRefreshEnabled = true private var hideFab = false @@ -174,7 +172,9 @@ class TimelineFragment : isSwipeToRefreshEnabled = arguments.getBoolean(ARG_ENABLE_SWIPE_TO_REFRESH, true) readingOrder = ReadingOrder.from(preferences.getString(PrefKeys.READING_ORDER, null)) + } + private fun createAdapter(): TimelinePagingAdapter { val statusDisplayOptions = StatusDisplayOptions( animateAvatars = preferences.getBoolean(PrefKeys.ANIMATE_GIF_AVATARS, false), mediaPreviewEnabled = accountManager.activeAccount!!.mediaPreviewEnabled, @@ -198,25 +198,20 @@ class TimelineFragment : showSensitiveMedia = accountManager.activeAccount!!.alwaysShowSensitiveMedia, openSpoiler = accountManager.activeAccount!!.alwaysOpenSpoiler ) - adapter = TimelinePagingAdapter( + return TimelinePagingAdapter( statusDisplayOptions, this ) } - override fun onCreateView( - inflater: LayoutInflater, - container: ViewGroup?, - savedInstanceState: Bundle? - ): View? { - return inflater.inflate(R.layout.fragment_timeline, container, false) - } - override fun onViewCreated(view: View, savedInstanceState: Bundle?) { requireActivity().addMenuProvider(this, viewLifecycleOwner, Lifecycle.State.RESUMED) + val adapter = createAdapter() + this.adapter = adapter + setupSwipeRefreshLayout() - setupRecyclerView() + setupRecyclerView(adapter) adapter.addLoadStateListener { loadState -> if (loadState.refresh != LoadState.Loading && loadState.source.refresh != LoadState.Loading) { @@ -272,7 +267,7 @@ class TimelineFragment : } } if (readingOrder == ReadingOrder.OLDEST_FIRST) { - updateReadingPositionForOldestFirst() + updateReadingPositionForOldestFirst(adapter) } } }) @@ -307,12 +302,12 @@ class TimelineFragment : eventHub.events.collect { event -> when (event) { is PreferenceChangedEvent -> { - onPreferenceChanged(event.preferenceKey) + onPreferenceChanged(adapter, event.preferenceKey) } is StatusComposedEvent -> { val status = event.status - handleStatusComposeEvent(status) + handleStatusComposeEvent(adapter, status) } } } @@ -327,6 +322,12 @@ class TimelineFragment : } } + override fun onDestroyView() { + // Clear the adapter to prevent leaking the View + adapter = null + super.onDestroyView() + } + override fun onCreateMenu(menu: Menu, menuInflater: MenuInflater) { if (isSwipeToRefreshEnabled) { menuInflater.inflate(R.menu.fragment_timeline, menu) @@ -365,7 +366,7 @@ class TimelineFragment : // match the adapter position where data was inserted (which is why loadMorePosition // is tracked manually, see this bug report for another example: // https://github.com/android/architecture-components-samples/issues/726). - private fun updateReadingPositionForOldestFirst() { + private fun updateReadingPositionForOldestFirst(adapter: TimelinePagingAdapter) { var position = loadMorePosition ?: return val statusIdBelowLoadMore = statusIdBelowLoadMore ?: return @@ -394,7 +395,7 @@ class TimelineFragment : binding.swipeRefreshLayout.setColorSchemeResources(R.color.tusky_blue) } - private fun setupRecyclerView() { + private fun setupRecyclerView(adapter: TimelinePagingAdapter) { binding.recyclerView.setAccessibilityDelegateCompat( ListStatusAccessibilityDelegate(binding.recyclerView, this) { pos -> if (pos in 0 until adapter.itemCount) { @@ -417,7 +418,7 @@ class TimelineFragment : override fun onRefresh() { binding.statusView.hide() - adapter.refresh() + adapter?.refresh() } override val onMoreTranslate = @@ -432,18 +433,18 @@ class TimelineFragment : } override fun onReply(position: Int) { - val status = adapter.peek(position)?.asStatusOrNull() ?: return + val status = adapter?.peek(position)?.asStatusOrNull() ?: return super.reply(status.status) } override fun onReblog(reblog: Boolean, position: Int) { - val status = adapter.peek(position)?.asStatusOrNull() ?: return + val status = adapter?.peek(position)?.asStatusOrNull() ?: return viewModel.reblog(reblog, status) } private fun onTranslate(position: Int) { - val status = adapter.peek(position)?.asStatusOrNull() ?: return - lifecycleScope.launch { + val status = adapter?.peek(position)?.asStatusOrNull() ?: return + viewLifecycleOwner.lifecycleScope.launch { viewModel.translate(status) .onFailure { Snackbar.make( @@ -456,32 +457,32 @@ class TimelineFragment : } override fun onUntranslate(position: Int) { - val status = adapter.peek(position)?.asStatusOrNull() ?: return + val status = adapter?.peek(position)?.asStatusOrNull() ?: return viewModel.untranslate(status) } override fun onFavourite(favourite: Boolean, position: Int) { - val status = adapter.peek(position)?.asStatusOrNull() ?: return + val status = adapter?.peek(position)?.asStatusOrNull() ?: return viewModel.favorite(favourite, status) } override fun onBookmark(bookmark: Boolean, position: Int) { - val status = adapter.peek(position)?.asStatusOrNull() ?: return + val status = adapter?.peek(position)?.asStatusOrNull() ?: return viewModel.bookmark(bookmark, status) } override fun onVoteInPoll(position: Int, choices: List) { - val status = adapter.peek(position)?.asStatusOrNull() ?: return + val status = adapter?.peek(position)?.asStatusOrNull() ?: return viewModel.voteInPoll(choices, status) } override fun clearWarningAction(position: Int) { - val status = adapter.peek(position)?.asStatusOrNull() ?: return + val status = adapter?.peek(position)?.asStatusOrNull() ?: return viewModel.clearWarning(status) } override fun onMore(view: View, position: Int) { - val status = adapter.peek(position)?.asStatusOrNull() ?: return + val status = adapter?.peek(position)?.asStatusOrNull() ?: return super.more( status.status, view, @@ -491,34 +492,35 @@ class TimelineFragment : } override fun onOpenReblog(position: Int) { - val status = adapter.peek(position)?.asStatusOrNull() ?: return + val status = adapter?.peek(position)?.asStatusOrNull() ?: return super.openReblog(status.status) } override fun onExpandedChange(expanded: Boolean, position: Int) { - val status = adapter.peek(position)?.asStatusOrNull() ?: return + val status = adapter?.peek(position)?.asStatusOrNull() ?: return viewModel.changeExpanded(expanded, status) } override fun onContentHiddenChange(isShowing: Boolean, position: Int) { - val status = adapter.peek(position)?.asStatusOrNull() ?: return + val status = adapter?.peek(position)?.asStatusOrNull() ?: return viewModel.changeContentShowing(isShowing, status) } override fun onShowReblogs(position: Int) { - val statusId = adapter.peek(position)?.asStatusOrNull()?.id ?: return + val statusId = adapter?.peek(position)?.asStatusOrNull()?.id ?: return val intent = newIntent(requireContext(), AccountListActivity.Type.REBLOGGED, statusId) activity?.startActivityWithSlideInAnimation(intent) } override fun onShowFavs(position: Int) { - val statusId = adapter.peek(position)?.asStatusOrNull()?.id ?: return + val statusId = adapter?.peek(position)?.asStatusOrNull()?.id ?: return val intent = newIntent(requireContext(), AccountListActivity.Type.FAVOURITED, statusId) activity?.startActivityWithSlideInAnimation(intent) } override fun onLoadMore(position: Int) { - val placeholder = adapter.peek(position)?.asPlaceholderOrNull() ?: return + val adapter = this.adapter + val placeholder = adapter?.peek(position)?.asPlaceholderOrNull() ?: return loadMorePosition = position statusIdBelowLoadMore = if (position + 1 < adapter.itemCount) adapter.peek(position + 1)?.id else null @@ -526,12 +528,12 @@ class TimelineFragment : } override fun onContentCollapsedChange(isCollapsed: Boolean, position: Int) { - val status = adapter.peek(position)?.asStatusOrNull() ?: return + val status = adapter?.peek(position)?.asStatusOrNull() ?: return viewModel.changeContentCollapsed(isCollapsed, status) } override fun onViewMedia(position: Int, attachmentIndex: Int, view: View?) { - val status = adapter.peek(position)?.asStatusOrNull() ?: return + val status = adapter?.peek(position)?.asStatusOrNull() ?: return super.viewMedia( attachmentIndex, AttachmentViewData.list(status), @@ -540,7 +542,7 @@ class TimelineFragment : } override fun onViewThread(position: Int) { - val status = adapter.peek(position)?.asStatusOrNull() ?: return + val status = adapter?.peek(position)?.asStatusOrNull() ?: return super.viewThread(status.actionable.id, status.actionable.url) } @@ -568,7 +570,7 @@ class TimelineFragment : super.viewAccount(id) } - private fun onPreferenceChanged(key: String) { + private fun onPreferenceChanged(adapter: TimelinePagingAdapter, key: String) { when (key) { PrefKeys.FAB_HIDE -> { hideFab = preferences.getBoolean(PrefKeys.FAB_HIDE, false) @@ -591,7 +593,7 @@ class TimelineFragment : } } - private fun handleStatusComposeEvent(status: Status) { + private fun handleStatusComposeEvent(adapter: TimelinePagingAdapter, status: Status) { when (kind) { TimelineViewModel.Kind.HOME, TimelineViewModel.Kind.PUBLIC_FEDERATED, @@ -612,7 +614,7 @@ class TimelineFragment : } public override fun removeItem(position: Int) { - val status = adapter.peek(position)?.asStatusOrNull() ?: return + val status = adapter?.peek(position)?.asStatusOrNull() ?: return viewModel.removeStatusWithId(status.id) } @@ -630,7 +632,7 @@ class TimelineFragment : (binding.recyclerView.layoutManager as? LinearLayoutManager)?.findFirstVisibleItemPosition() ?.let { position -> if (position != RecyclerView.NO_POSITION) { - adapter.snapshot().getOrNull(position)?.id?.let { statusId -> + adapter?.snapshot()?.getOrNull(position)?.id?.let { statusId -> viewModel.saveReadingPosition(statusId) } } @@ -639,19 +641,19 @@ class TimelineFragment : override fun onResume() { super.onResume() - val a11yManager = - ContextCompat.getSystemService(requireContext(), AccessibilityManager::class.java) + val a11yManager = requireContext().getSystemService() val wasEnabled = talkBackWasEnabled talkBackWasEnabled = a11yManager?.isEnabled == true Log.d(TAG, "talkback was enabled: $wasEnabled, now $talkBackWasEnabled") if (talkBackWasEnabled && !wasEnabled) { + val adapter = requireNotNull(this.adapter) adapter.notifyItemRangeChanged(0, adapter.itemCount) } } override fun onReselect() { - if (isAdded) { + if (view != null) { binding.recyclerView.layoutManager?.scrollToPosition(0) binding.recyclerView.stopScroll() } diff --git a/app/src/main/java/com/keylesspalace/tusky/components/trending/TrendingTagsFragment.kt b/app/src/main/java/com/keylesspalace/tusky/components/trending/TrendingTagsFragment.kt index e746f4504..ba068469a 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/trending/TrendingTagsFragment.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/trending/TrendingTagsFragment.kt @@ -20,7 +20,7 @@ import android.os.Bundle import android.util.Log import android.view.View import android.view.accessibility.AccessibilityManager -import androidx.core.content.ContextCompat +import androidx.core.content.getSystemService import androidx.fragment.app.Fragment import androidx.fragment.app.viewModels import androidx.lifecycle.lifecycleScope @@ -57,18 +57,22 @@ class TrendingTagsFragment : private val binding by viewBinding(FragmentTrendingTagsBinding::bind) - private val adapter = TrendingTagsAdapter(::onViewTag) + private var adapter: TrendingTagsAdapter? = null override fun onConfigurationChanged(newConfig: Configuration) { super.onConfigurationChanged(newConfig) val columnCount = requireContext().resources.getInteger(R.integer.trending_column_count) - setupLayoutManager(columnCount) + adapter?.let { + setupLayoutManager(it, columnCount) + } } override fun onViewCreated(view: View, savedInstanceState: Bundle?) { + val adapter = TrendingTagsAdapter(::onViewTag) + this.adapter = adapter setupSwipeRefreshLayout() - setupRecyclerView() + setupRecyclerView(adapter) adapter.registerAdapterDataObserver(object : RecyclerView.AdapterDataObserver() { override fun onItemRangeInserted(positionStart: Int, itemCount: Int) { @@ -87,13 +91,17 @@ class TrendingTagsFragment : viewLifecycleOwner.lifecycleScope.launch { viewModel.uiState.collectLatest { trendingState -> - processViewState(trendingState) + processViewState(adapter, trendingState) } } - if (activity is ActionButtonActivity) { - (activity as ActionButtonActivity).actionButton?.visibility = View.GONE - } + (requireActivity() as? ActionButtonActivity)?.actionButton?.visibility = View.GONE + } + + override fun onDestroyView() { + // Clear the adapter to prevent leaking the View + adapter = null + super.onDestroyView() } private fun setupSwipeRefreshLayout() { @@ -101,7 +109,7 @@ class TrendingTagsFragment : binding.swipeRefreshLayout.setColorSchemeResources(R.color.tusky_blue) } - private fun setupLayoutManager(columnCount: Int) { + private fun setupLayoutManager(adapter: TrendingTagsAdapter, columnCount: Int) { binding.recyclerView.layoutManager = GridLayoutManager(context, columnCount).apply { spanSizeLookup = object : SpanSizeLookup() { override fun getSpanSize(position: Int): Int { @@ -115,10 +123,10 @@ class TrendingTagsFragment : } } - private fun setupRecyclerView() { + private fun setupRecyclerView(adapter: TrendingTagsAdapter) { val columnCount = requireContext().resources.getInteger(R.integer.trending_column_count) - setupLayoutManager(columnCount) + setupLayoutManager(adapter, columnCount) binding.recyclerView.setHasFixedSize(true) @@ -136,19 +144,22 @@ class TrendingTagsFragment : ) } - private fun processViewState(uiState: TrendingTagsViewModel.TrendingTagsUiState) { + private fun processViewState( + adapter: TrendingTagsAdapter, + uiState: TrendingTagsViewModel.TrendingTagsUiState + ) { Log.d(TAG, uiState.loadingState.name) when (uiState.loadingState) { TrendingTagsViewModel.LoadingState.INITIAL -> clearLoadingState() TrendingTagsViewModel.LoadingState.LOADING -> applyLoadingState() TrendingTagsViewModel.LoadingState.REFRESHING -> applyRefreshingState() - TrendingTagsViewModel.LoadingState.LOADED -> applyLoadedState(uiState.trendingViewData) + TrendingTagsViewModel.LoadingState.LOADED -> applyLoadedState(adapter, uiState.trendingViewData) TrendingTagsViewModel.LoadingState.ERROR_NETWORK -> networkError() TrendingTagsViewModel.LoadingState.ERROR_OTHER -> otherError() } } - private fun applyLoadedState(viewData: List) { + private fun applyLoadedState(adapter: TrendingTagsAdapter, viewData: List) { clearLoadingState() adapter.submitList(viewData) @@ -216,13 +227,13 @@ class TrendingTagsFragment : override fun onResume() { super.onResume() - val a11yManager = - ContextCompat.getSystemService(requireContext(), AccessibilityManager::class.java) + val a11yManager = requireContext().getSystemService() val wasEnabled = talkBackWasEnabled talkBackWasEnabled = a11yManager?.isEnabled == true Log.d(TAG, "talkback was enabled: $wasEnabled, now $talkBackWasEnabled") if (talkBackWasEnabled && !wasEnabled) { + val adapter = requireNotNull(this.adapter) adapter.notifyItemRangeChanged(0, adapter.itemCount) } @@ -233,7 +244,7 @@ class TrendingTagsFragment : } override fun onReselect() { - if (isAdded) { + if (view != null) { binding.recyclerView.layoutManager?.scrollToPosition(0) binding.recyclerView.stopScroll() } diff --git a/app/src/main/java/com/keylesspalace/tusky/components/viewthread/ViewThreadFragment.kt b/app/src/main/java/com/keylesspalace/tusky/components/viewthread/ViewThreadFragment.kt index 359438b10..abe1068ce 100644 --- a/app/src/main/java/com/keylesspalace/tusky/components/viewthread/ViewThreadFragment.kt +++ b/app/src/main/java/com/keylesspalace/tusky/components/viewthread/ViewThreadFragment.kt @@ -18,12 +18,10 @@ package com.keylesspalace.tusky.components.viewthread import android.content.SharedPreferences import android.os.Bundle import android.util.Log -import android.view.LayoutInflater import android.view.Menu import android.view.MenuInflater import android.view.MenuItem import android.view.View -import android.view.ViewGroup import android.widget.LinearLayout import androidx.annotation.CheckResult import androidx.core.view.MenuProvider @@ -65,7 +63,7 @@ import kotlinx.coroutines.launch @AndroidEntryPoint class ViewThreadFragment : - SFragment(), + SFragment(R.layout.fragment_view_thread), OnRefreshListener, StatusActionListener, MenuProvider { @@ -77,7 +75,7 @@ class ViewThreadFragment : private val binding by viewBinding(FragmentViewThreadBinding::bind) - private lateinit var adapter: ThreadAdapter + private var adapter: ThreadAdapter? = null private lateinit var thisThreadsStatusId: String private var alwaysShowSensitiveMedia = false @@ -96,7 +94,9 @@ class ViewThreadFragment : override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) thisThreadsStatusId = requireArguments().getString(ID_EXTRA)!! + } + private fun createAdapter(): ThreadAdapter { val statusDisplayOptions = StatusDisplayOptions( animateAvatars = preferences.getBoolean(PrefKeys.ANIMATE_GIF_AVATARS, false), mediaPreviewEnabled = accountManager.activeAccount!!.mediaPreviewEnabled, @@ -116,20 +116,15 @@ class ViewThreadFragment : showSensitiveMedia = accountManager.activeAccount!!.alwaysShowSensitiveMedia, openSpoiler = accountManager.activeAccount!!.alwaysOpenSpoiler ) - adapter = ThreadAdapter(statusDisplayOptions, this) - } - - override fun onCreateView( - inflater: LayoutInflater, - container: ViewGroup?, - savedInstanceState: Bundle? - ): View? { - return inflater.inflate(R.layout.fragment_view_thread, container, false) + return ThreadAdapter(statusDisplayOptions, this) } override fun onViewCreated(view: View, savedInstanceState: Bundle?) { requireActivity().addMenuProvider(this, viewLifecycleOwner, Lifecycle.State.RESUMED) + val adapter = createAdapter() + this.adapter = adapter + binding.swipeRefreshLayout.setOnRefreshListener(this) binding.swipeRefreshLayout.setColorSchemeResources(R.color.tusky_blue) @@ -256,6 +251,12 @@ class ViewThreadFragment : viewModel.loadThread(thisThreadsStatusId) } + override fun onDestroyView() { + // Clear the adapter to prevent leaking the View + adapter = null + super.onDestroyView() + } + override fun onCreateMenu(menu: Menu, menuInflater: MenuInflater) { menuInflater.inflate(R.menu.fragment_view_thread, menu) val actionReveal = menu.findItem(R.id.action_reveal) @@ -322,11 +323,12 @@ class ViewThreadFragment : } override fun onReply(position: Int) { - super.reply(adapter.currentList[position].status) + val viewData = adapter?.currentList?.getOrNull(position) ?: return + super.reply(viewData.status) } override fun onReblog(reblog: Boolean, position: Int) { - val status = adapter.currentList[position] + val status = adapter?.currentList?.getOrNull(position) ?: return viewModel.reblog(reblog, status) } @@ -342,8 +344,8 @@ class ViewThreadFragment : } private fun onTranslate(position: Int) { - val status = adapter.currentList[position] - lifecycleScope.launch { + val status = adapter?.currentList?.getOrNull(position) ?: return + viewLifecycleOwner.lifecycleScope.launch { viewModel.translate(status) .onFailure { Snackbar.make( @@ -356,22 +358,22 @@ class ViewThreadFragment : } override fun onUntranslate(position: Int) { - val status = adapter.currentList[position] + val status = adapter?.currentList?.getOrNull(position) ?: return viewModel.untranslate(status) } override fun onFavourite(favourite: Boolean, position: Int) { - val status = adapter.currentList[position] + val status = adapter?.currentList?.getOrNull(position) ?: return viewModel.favorite(favourite, status) } override fun onBookmark(bookmark: Boolean, position: Int) { - val status = adapter.currentList[position] + val status = adapter?.currentList?.getOrNull(position) ?: return viewModel.bookmark(bookmark, status) } override fun onMore(view: View, position: Int) { - val viewData = adapter.currentList[position] + val viewData = adapter?.currentList?.getOrNull(position) ?: return super.more( viewData.status, view, @@ -381,7 +383,7 @@ class ViewThreadFragment : } override fun onViewMedia(position: Int, attachmentIndex: Int, view: View?) { - val status = adapter.currentList[position] + val status = adapter?.currentList?.getOrNull(position) ?: return super.viewMedia( attachmentIndex, list(status, alwaysShowSensitiveMedia), @@ -390,7 +392,7 @@ class ViewThreadFragment : } override fun onViewThread(position: Int) { - val status = adapter.currentList[position] + val status = adapter?.currentList?.getOrNull(position) ?: return if (thisThreadsStatusId == status.id) { // If already viewing this thread, don't reopen it. return @@ -415,11 +417,13 @@ class ViewThreadFragment : } override fun onExpandedChange(expanded: Boolean, position: Int) { - viewModel.changeExpanded(expanded, adapter.currentList[position]) + val status = adapter?.currentList?.getOrNull(position) ?: return + viewModel.changeExpanded(expanded, status) } override fun onContentHiddenChange(isShowing: Boolean, position: Int) { - viewModel.changeContentShowing(isShowing, adapter.currentList[position]) + val status = adapter?.currentList?.getOrNull(position) ?: return + viewModel.changeContentShowing(isShowing, status) } override fun onLoadMore(position: Int) { @@ -427,19 +431,20 @@ class ViewThreadFragment : } override fun onShowReblogs(position: Int) { - val statusId = adapter.currentList[position].id - val intent = newIntent(requireContext(), AccountListActivity.Type.REBLOGGED, statusId) + val status = adapter?.currentList?.getOrNull(position) ?: return + val intent = newIntent(requireContext(), AccountListActivity.Type.REBLOGGED, status.id) requireActivity().startActivityWithSlideInAnimation(intent) } override fun onShowFavs(position: Int) { - val statusId = adapter.currentList[position].id - val intent = newIntent(requireContext(), AccountListActivity.Type.FAVOURITED, statusId) + val status = adapter?.currentList?.getOrNull(position) ?: return + val intent = newIntent(requireContext(), AccountListActivity.Type.FAVOURITED, status.id) requireActivity().startActivityWithSlideInAnimation(intent) } override fun onContentCollapsedChange(isCollapsed: Boolean, position: Int) { - viewModel.changeContentCollapsed(isCollapsed, adapter.currentList[position]) + val status = adapter?.currentList?.getOrNull(position) ?: return + viewModel.changeContentCollapsed(isCollapsed, status) } override fun onViewTag(tag: String) { @@ -451,7 +456,7 @@ class ViewThreadFragment : } public override fun removeItem(position: Int) { - adapter.currentList.getOrNull(position)?.let { status -> + adapter?.currentList?.getOrNull(position)?.let { status -> if (status.isDetailed) { // the main status we are viewing is being removed, finish the activity activity?.finish() @@ -462,12 +467,12 @@ class ViewThreadFragment : } override fun onVoteInPoll(position: Int, choices: List) { - val status = adapter.currentList[position] + val status = adapter?.currentList?.getOrNull(position) ?: return viewModel.voteInPoll(choices, status) } override fun onShowEdits(position: Int) { - val status = adapter.currentList[position] + val status = adapter?.currentList?.getOrNull(position) ?: return val viewEditsFragment = ViewEditsFragment.newInstance(status.actionableId) parentFragmentManager.commit { @@ -483,7 +488,8 @@ class ViewThreadFragment : } override fun clearWarningAction(position: Int) { - viewModel.clearWarning(adapter.currentList[position]) + val status = adapter?.currentList?.getOrNull(position) ?: return + viewModel.clearWarning(status) } companion object { diff --git a/app/src/main/java/com/keylesspalace/tusky/fragment/SFragment.kt b/app/src/main/java/com/keylesspalace/tusky/fragment/SFragment.kt index e9e147fa0..26529763a 100644 --- a/app/src/main/java/com/keylesspalace/tusky/fragment/SFragment.kt +++ b/app/src/main/java/com/keylesspalace/tusky/fragment/SFragment.kt @@ -30,6 +30,7 @@ import android.view.MenuItem import android.view.View import android.widget.Toast import androidx.activity.result.contract.ActivityResultContracts +import androidx.annotation.LayoutRes import androidx.appcompat.app.AlertDialog import androidx.appcompat.widget.PopupMenu import androidx.core.app.ActivityOptionsCompat @@ -73,14 +74,16 @@ import kotlinx.coroutines.launch * adapters. I feel like the profile pages and thread viewer, which I haven't made yet, will also * overlap functionality. So, I'm momentarily leaving it and hopefully working on those will clear * up what needs to be where. */ -abstract class SFragment : Fragment() { +abstract class SFragment(@LayoutRes contentLayoutId: Int) : Fragment(contentLayoutId) { protected abstract fun removeItem(position: Int) protected abstract fun onReblog(reblog: Boolean, position: Int) /** `null` if translation is not supported on this screen */ protected abstract val onMoreTranslate: ((translate: Boolean, position: Int) -> Unit)? - private lateinit var bottomSheetActivity: BottomSheetActivity + private val bottomSheetActivity: BottomSheetActivity + get() = (requireActivity() as? BottomSheetActivity) + ?: throw IllegalStateException("Fragment must be attached to a BottomSheetActivity!") @Inject lateinit var mastodonApi: MastodonApi @@ -114,15 +117,6 @@ abstract class SFragment : Fragment() { requireActivity().startActivityWithSlideInAnimation(intent) } - override fun onAttach(context: Context) { - super.onAttach(context) - bottomSheetActivity = if (context is BottomSheetActivity) { - context - } else { - throw IllegalStateException("Fragment must be attached to a BottomSheetActivity!") - } - } - override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) pendingMediaDownloads = savedInstanceState?.getStringArrayList(PENDING_MEDIA_DOWNLOADS_STATE_KEY) @@ -357,7 +351,7 @@ abstract class SFragment : Fragment() { } R.id.pin -> { - lifecycleScope.launch { + viewLifecycleOwner.lifecycleScope.launch { timelineCases.pin(status.id, !status.pinned) .onFailure { e: Throwable -> val message = e.message @@ -389,9 +383,9 @@ abstract class SFragment : Fragment() { showMuteAccountDialog( this.requireActivity(), accountUsername - ) { notifications: Boolean?, duration: Int? -> + ) { notifications: Boolean, duration: Int? -> lifecycleScope.launch { - timelineCases.mute(accountId, notifications == true, duration) + timelineCases.mute(accountId, notifications, duration) } } } @@ -445,11 +439,11 @@ abstract class SFragment : Fragment() { AlertDialog.Builder(requireActivity()) .setMessage(R.string.dialog_delete_post_warning) .setPositiveButton(android.R.string.ok) { _: DialogInterface?, _: Int -> - lifecycleScope.launch { + viewLifecycleOwner.lifecycleScope.launch { val result = timelineCases.delete(id).exceptionOrNull() if (result != null) { Log.w("SFragment", "error deleting status", result) - Toast.makeText(context, R.string.error_generic, Toast.LENGTH_SHORT).show() + Toast.makeText(requireContext(), R.string.error_generic, Toast.LENGTH_SHORT).show() } // XXX: Removes the item even if there was an error. This is probably not // correct (see similar code in showConfirmEditDialog() which only @@ -464,13 +458,12 @@ abstract class SFragment : Fragment() { } private fun showConfirmEditDialog(id: String, position: Int, status: Status) { - if (activity == null) { - return - } - AlertDialog.Builder(requireActivity()) + val context = context ?: return + + AlertDialog.Builder(context) .setMessage(R.string.dialog_redraft_post_warning) .setPositiveButton(android.R.string.ok) { _: DialogInterface?, _: Int -> - lifecycleScope.launch { + viewLifecycleOwner.lifecycleScope.launch { timelineCases.delete(id).fold( { deletedStatus -> removeItem(position) @@ -491,7 +484,7 @@ abstract class SFragment : Fragment() { poll = sourceStatus.poll?.toNewPoll(sourceStatus.createdAt), kind = ComposeActivity.ComposeKind.NEW ) - startActivity(startIntent(requireContext(), composeOptions)) + startActivity(startIntent(context, composeOptions)) }, { error: Throwable? -> Log.w("SFragment", "error deleting status", error) @@ -506,7 +499,7 @@ abstract class SFragment : Fragment() { } private fun editStatus(id: String, status: Status) { - lifecycleScope.launch { + viewLifecycleOwner.lifecycleScope.launch { mastodonApi.statusSource(id).fold( { source -> val composeOptions = ComposeOptions( diff --git a/app/src/main/java/com/keylesspalace/tusky/fragment/ViewImageFragment.kt b/app/src/main/java/com/keylesspalace/tusky/fragment/ViewImageFragment.kt index 0aa18781d..9a62e1cc7 100644 --- a/app/src/main/java/com/keylesspalace/tusky/fragment/ViewImageFragment.kt +++ b/app/src/main/java/com/keylesspalace/tusky/fragment/ViewImageFragment.kt @@ -18,7 +18,6 @@ package com.keylesspalace.tusky.fragment import android.animation.Animator import android.animation.AnimatorListenerAdapter import android.annotation.SuppressLint -import android.content.Context import android.graphics.PointF import android.graphics.drawable.Drawable import android.os.Bundle @@ -35,7 +34,6 @@ import com.bumptech.glide.load.engine.GlideException import com.bumptech.glide.request.RequestListener import com.bumptech.glide.request.target.Target import com.keylesspalace.tusky.R -import com.keylesspalace.tusky.ViewMediaActivity import com.keylesspalace.tusky.databinding.FragmentViewImageBinding import com.keylesspalace.tusky.entity.Attachment import com.keylesspalace.tusky.util.getParcelableCompat @@ -57,8 +55,8 @@ class ViewImageFragment : ViewMediaFragment() { private val binding by viewBinding(FragmentViewImageBinding::bind) - private lateinit var photoActionsListener: PhotoActionsListener - private lateinit var toolbar: View + private val photoActionsListener: PhotoActionsListener + get() = requireActivity() as PhotoActionsListener private var transition: CompletableDeferred? = null private var shouldStartTransition = false @@ -67,11 +65,6 @@ class ViewImageFragment : ViewMediaFragment() { @Volatile private var startedTransition = false - override fun onAttach(context: Context) { - super.onAttach(context) - photoActionsListener = context as PhotoActionsListener - } - override fun setupMediaView( url: String, previewUrl: String?, @@ -91,7 +84,6 @@ class ViewImageFragment : ViewMediaFragment() { container: ViewGroup?, savedInstanceState: Bundle? ): View { - toolbar = (requireActivity() as ViewMediaActivity).toolbar this.transition = CompletableDeferred() return inflater.inflate(R.layout.fragment_view_image, container, false) } diff --git a/app/src/main/java/com/keylesspalace/tusky/fragment/ViewVideoFragment.kt b/app/src/main/java/com/keylesspalace/tusky/fragment/ViewVideoFragment.kt index 2fbb85f00..675256241 100644 --- a/app/src/main/java/com/keylesspalace/tusky/fragment/ViewVideoFragment.kt +++ b/app/src/main/java/com/keylesspalace/tusky/fragment/ViewVideoFragment.kt @@ -18,7 +18,6 @@ package com.keylesspalace.tusky.fragment import android.animation.Animator import android.animation.AnimatorListenerAdapter import android.annotation.SuppressLint -import android.content.Context import android.graphics.drawable.Drawable import android.os.Bundle import android.os.Handler @@ -33,6 +32,8 @@ import android.view.ViewGroup import android.widget.FrameLayout import android.widget.LinearLayout import androidx.annotation.OptIn +import androidx.lifecycle.DefaultLifecycleObserver +import androidx.lifecycle.LifecycleOwner import androidx.media3.common.AudioAttributes import androidx.media3.common.C import androidx.media3.common.MediaItem @@ -54,6 +55,7 @@ import com.keylesspalace.tusky.databinding.FragmentViewVideoBinding import com.keylesspalace.tusky.entity.Attachment import com.keylesspalace.tusky.util.getParcelableCompat import com.keylesspalace.tusky.util.hide +import com.keylesspalace.tusky.util.unsafeLazy import com.keylesspalace.tusky.util.viewBinding import com.keylesspalace.tusky.util.visible import dagger.hilt.android.AndroidEntryPoint @@ -73,19 +75,23 @@ class ViewVideoFragment : ViewMediaFragment() { private val binding by viewBinding(FragmentViewVideoBinding::bind) - private lateinit var videoActionsListener: VideoActionsListener - private lateinit var toolbar: View + private val videoActionsListener: VideoActionsListener + get() = requireActivity() as VideoActionsListener private val handler = Handler(Looper.getMainLooper()) private val hideToolbar = Runnable { // Hoist toolbar hiding to activity so it can track state across different fragments // This is explicitly stored as runnable so that we pass it to the handler later for cancellation mediaActivity.onPhotoTap() } - private lateinit var mediaActivity: ViewMediaActivity - private lateinit var mediaPlayerListener: Player.Listener - private var isAudio = false + private val mediaActivity: ViewMediaActivity + get() = requireActivity() as ViewMediaActivity + private val isAudio + get() = mediaAttachment.type == Attachment.Type.AUDIO - private lateinit var mediaAttachment: Attachment + private val mediaAttachment: Attachment by unsafeLazy { + arguments?.getParcelableCompat(ARG_ATTACHMENT) + ?: throw IllegalArgumentException("attachment has to be set") + } private var player: ExoPlayer? = null @@ -101,20 +107,12 @@ class ViewVideoFragment : ViewMediaFragment() { /** Prevent the next play start from queueing a toolbar hide. */ private var suppressNextHideToolbar = false - override fun onAttach(context: Context) { - super.onAttach(context) - - videoActionsListener = context as VideoActionsListener - } - @SuppressLint("PrivateResource", "MissingInflatedId") override fun onCreateView( inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle? ): View { - mediaActivity = activity as ViewMediaActivity - toolbar = mediaActivity.toolbar val rootView = inflater.inflate(R.layout.fragment_view_video, container, false) // Move the controls to the bottom of the screen, with enough bottom margin to clear the seekbar @@ -133,11 +131,6 @@ class ViewVideoFragment : ViewMediaFragment() { @SuppressLint("ClickableViewAccessibility") override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) - val attachment = arguments?.getParcelableCompat(ARG_ATTACHMENT) - ?: throw IllegalArgumentException("attachment has to be set") - - val url = attachment.url - isAudio = attachment.type == Attachment.Type.AUDIO /** * Handle single taps, flings, and dragging @@ -211,7 +204,7 @@ class ViewVideoFragment : ViewMediaFragment() { } } - mediaPlayerListener = object : Player.Listener { + val mediaPlayerListener = object : Player.Listener { @SuppressLint("ClickableViewAccessibility", "SyntheticAccessor") @OptIn(UnstableApi::class) override fun onPlaybackStateChanged(playbackState: Int) { @@ -273,25 +266,23 @@ class ViewVideoFragment : ViewMediaFragment() { savedSeekPosition = savedInstanceState?.getLong(SEEK_POSITION) ?: 0 - mediaAttachment = attachment + val attachment = mediaAttachment + finalizeViewSetup(attachment.url, attachment.previewUrl, attachment.description) - finalizeViewSetup(url, attachment.previewUrl, attachment.description) - } + // Lifecycle callbacks + viewLifecycleOwner.lifecycle.addObserver(object : DefaultLifecycleObserver { + override fun onStart(owner: LifecycleOwner) { + initializePlayer(mediaPlayerListener) + binding.videoView.onResume() + } - override fun onStart() { - super.onStart() - - initializePlayer() - binding.videoView.onResume() - } - - override fun onStop() { - super.onStop() - - // This might be multi-window, so pause everything now. - binding.videoView.onPause() - releasePlayer() - handler.removeCallbacks(hideToolbar) + override fun onStop(owner: LifecycleOwner) { + // This might be multi-window, so pause everything now. + binding.videoView.onPause() + releasePlayer() + handler.removeCallbacks(hideToolbar) + } + }) } override fun onSaveInstanceState(outState: Bundle) { @@ -299,7 +290,7 @@ class ViewVideoFragment : ViewMediaFragment() { outState.putLong(SEEK_POSITION, savedSeekPosition) } - private fun initializePlayer() { + private fun initializePlayer(mediaPlayerListener: Player.Listener) { player = playerProvider.get().apply { setAudioAttributes( AudioAttributes.Builder()