From 965d51100c4f772a99950d3e519be06b4dca46f7 Mon Sep 17 00:00:00 2001 From: Nik Clayton Date: Mon, 5 Dec 2022 14:51:45 +0100 Subject: [PATCH] Cleanup NotificationFragment to make future conversion to Kotlin easier (#2993) * Convert NotificationsFragment to use view binding * Use requireContext() in places a context is required Removes a nullness warning. * Simplify code by using .sublist() and .contains() Removes a lint warning. * Add @NonNull annotations to onViewTag and onViewAccount * Use consistent comment styles --- .../tusky/fragment/NotificationsFragment.java | 180 +++++++++--------- 1 file changed, 85 insertions(+), 95 deletions(-) diff --git a/app/src/main/java/com/keylesspalace/tusky/fragment/NotificationsFragment.java b/app/src/main/java/com/keylesspalace/tusky/fragment/NotificationsFragment.java index 8ee0bf01..99543b80 100644 --- a/app/src/main/java/com/keylesspalace/tusky/fragment/NotificationsFragment.java +++ b/app/src/main/java/com/keylesspalace/tusky/fragment/NotificationsFragment.java @@ -30,10 +30,8 @@ import android.view.LayoutInflater; import android.view.View; import android.view.ViewGroup; import android.widget.ArrayAdapter; -import android.widget.Button; import android.widget.ListView; import android.widget.PopupWindow; -import android.widget.ProgressBar; import androidx.annotation.NonNull; import androidx.annotation.Nullable; @@ -65,6 +63,7 @@ import com.keylesspalace.tusky.appstore.FavoriteEvent; import com.keylesspalace.tusky.appstore.PinEvent; import com.keylesspalace.tusky.appstore.PreferenceChangedEvent; import com.keylesspalace.tusky.appstore.ReblogEvent; +import com.keylesspalace.tusky.databinding.FragmentTimelineNotificationsBinding; import com.keylesspalace.tusky.db.AccountEntity; import com.keylesspalace.tusky.db.AccountManager; import com.keylesspalace.tusky.di.Injectable; @@ -87,7 +86,6 @@ import com.keylesspalace.tusky.util.NotificationTypeConverterKt; import com.keylesspalace.tusky.util.PairedList; import com.keylesspalace.tusky.util.StatusDisplayOptions; import com.keylesspalace.tusky.util.ViewDataUtils; -import com.keylesspalace.tusky.view.BackgroundMessageView; import com.keylesspalace.tusky.view.EndlessOnScrollListener; import com.keylesspalace.tusky.viewdata.AttachmentViewData; import com.keylesspalace.tusky.viewdata.NotificationViewData; @@ -158,16 +156,11 @@ public class NotificationsFragment extends SFragment implements @Inject EventHub eventHub; - private SwipeRefreshLayout swipeRefreshLayout; - private RecyclerView recyclerView; - private ProgressBar progressBar; - private BackgroundMessageView statusView; - private AppBarLayout appBarOptions; + private FragmentTimelineNotificationsBinding binding; private LinearLayoutManager layoutManager; private EndlessOnScrollListener scrollListener; private NotificationsAdapter adapter; - private Button buttonFilter; private boolean hideFab; private boolean topLoading; private boolean bottomLoading; @@ -211,35 +204,29 @@ public class NotificationsFragment extends SFragment implements @Override public View onCreateView(@NonNull LayoutInflater inflater, @Nullable ViewGroup container, @Nullable Bundle savedInstanceState) { - View rootView = inflater.inflate(R.layout.fragment_timeline_notifications, container, false); + binding = FragmentTimelineNotificationsBinding.inflate(inflater, container, false); @NonNull Context context = inflater.getContext(); // from inflater to silence warning - SharedPreferences preferences = PreferenceManager.getDefaultSharedPreferences(getActivity()); + SharedPreferences preferences = PreferenceManager.getDefaultSharedPreferences(requireContext()); boolean showNotificationsFilterSetting = preferences.getBoolean("showNotificationsFilter", true); - //Clear notifications on filter visibility change to force refresh + // Clear notifications on filter visibility change to force refresh if (showNotificationsFilterSetting != showNotificationsFilter) notifications.clear(); showNotificationsFilter = showNotificationsFilterSetting; // Setup the SwipeRefreshLayout. - swipeRefreshLayout = rootView.findViewById(R.id.swipeRefreshLayout); - recyclerView = rootView.findViewById(R.id.recyclerView); - progressBar = rootView.findViewById(R.id.progressBar); - statusView = rootView.findViewById(R.id.statusView); - appBarOptions = rootView.findViewById(R.id.appBarOptions); - - swipeRefreshLayout.setOnRefreshListener(this); - swipeRefreshLayout.setColorSchemeResources(R.color.tusky_blue); + binding.swipeRefreshLayout.setOnRefreshListener(this); + binding.swipeRefreshLayout.setColorSchemeResources(R.color.tusky_blue); loadNotificationsFilter(); // Setup the RecyclerView. - recyclerView.setHasFixedSize(true); + binding.recyclerView.setHasFixedSize(true); layoutManager = new LinearLayoutManager(context); - recyclerView.setLayoutManager(layoutManager); - recyclerView.setAccessibilityDelegateCompat( - new ListStatusAccessibilityDelegate(recyclerView, this, (pos) -> { + binding.recyclerView.setLayoutManager(layoutManager); + binding.recyclerView.setAccessibilityDelegateCompat( + new ListStatusAccessibilityDelegate(binding.recyclerView, this, (pos) -> { NotificationViewData notification = notifications.getPairedItemOrNull(pos); // We support replies only for now if (notification instanceof NotificationViewData.Concrete) { @@ -249,7 +236,7 @@ public class NotificationsFragment extends SFragment implements } })); - recyclerView.addItemDecoration(new DividerItemDecoration(context, DividerItemDecoration.VERTICAL)); + binding.recyclerView.addItemDecoration(new DividerItemDecoration(context, DividerItemDecoration.VERTICAL)); StatusDisplayOptions statusDisplayOptions = new StatusDisplayOptions( preferences.getBoolean("animateGifAvatars", false), @@ -268,7 +255,7 @@ public class NotificationsFragment extends SFragment implements dataSource, statusDisplayOptions, this, this, this); alwaysShowSensitiveMedia = accountManager.getActiveAccount().getAlwaysShowSensitiveMedia(); alwaysOpenSpoiler = accountManager.getActiveAccount().getAlwaysOpenSpoiler(); - recyclerView.setAdapter(adapter); + binding.recyclerView.setAdapter(adapter); topLoading = false; bottomLoading = false; @@ -276,43 +263,47 @@ public class NotificationsFragment extends SFragment implements updateAdapter(); - Button buttonClear = rootView.findViewById(R.id.buttonClear); - buttonClear.setOnClickListener(v -> confirmClearNotifications()); - buttonFilter = rootView.findViewById(R.id.buttonFilter); - buttonFilter.setOnClickListener(v -> showFilterMenu()); + binding.buttonClear.setOnClickListener(v -> confirmClearNotifications()); + binding.buttonFilter.setOnClickListener(v -> showFilterMenu()); if (notifications.isEmpty()) { - swipeRefreshLayout.setEnabled(false); + binding.swipeRefreshLayout.setEnabled(false); sendFetchNotificationsRequest(null, null, FetchEnd.BOTTOM, -1); } else { - progressBar.setVisibility(View.GONE); + binding.progressBar.setVisibility(View.GONE); } - ((SimpleItemAnimator) recyclerView.getItemAnimator()).setSupportsChangeAnimations(false); + ((SimpleItemAnimator) binding.recyclerView.getItemAnimator()).setSupportsChangeAnimations(false); updateFilterVisibility(); - return rootView; + return binding.getRoot(); + } + + @Override + public void onDestroyView() { + super.onDestroyView(); + binding = null; } private void updateFilterVisibility() { CoordinatorLayout.LayoutParams params = - (CoordinatorLayout.LayoutParams) swipeRefreshLayout.getLayoutParams(); + (CoordinatorLayout.LayoutParams) binding.swipeRefreshLayout.getLayoutParams(); if (showNotificationsFilter && !showingError) { - appBarOptions.setExpanded(true, false); - appBarOptions.setVisibility(View.VISIBLE); - //Set content behaviour to hide filter on scroll + binding.appBarOptions.setExpanded(true, false); + binding.appBarOptions.setVisibility(View.VISIBLE); + // Set content behaviour to hide filter on scroll params.setBehavior(new AppBarLayout.ScrollingViewBehavior()); } else { - appBarOptions.setExpanded(false, false); - appBarOptions.setVisibility(View.GONE); - //Clear behaviour to hide app bar + binding.appBarOptions.setExpanded(false, false); + binding.appBarOptions.setVisibility(View.GONE); + // Clear behaviour to hide app bar params.setBehavior(null); } } private void confirmClearNotifications() { - new AlertDialog.Builder(getContext()) + new AlertDialog.Builder(requireContext()) .setMessage(R.string.notification_clear_text) .setPositiveButton(android.R.string.ok, (DialogInterface dia, int which) -> clearNotifications()) .setNegativeButton(android.R.string.cancel, null) @@ -325,10 +316,10 @@ public class NotificationsFragment extends SFragment implements Activity activity = getActivity(); if (activity == null) throw new AssertionError("Activity is null"); - /* This is delayed until onActivityCreated solely because MainActivity.composeButton isn't - * guaranteed to be set until then. - * Use a modified scroll listener that both loads more notificationsEnabled as it goes, and hides - * the compose button on down-scroll. */ + // This is delayed until onActivityCreated solely because MainActivity.composeButton + // isn't guaranteed to be set until then. + // Use a modified scroll listener that both loads more notificationsEnabled as it + // goes, and hides the compose button on down-scroll. SharedPreferences preferences = PreferenceManager.getDefaultSharedPreferences(activity); hideFab = preferences.getBoolean("fabHide", false); scrollListener = new EndlessOnScrollListener(layoutManager) { @@ -342,9 +333,9 @@ public class NotificationsFragment extends SFragment implements if (composeButton != null) { if (hideFab) { if (dy > 0 && composeButton.isShown()) { - composeButton.hide(); // hides the button if we're scrolling down + composeButton.hide(); // Hides the button if we're scrolling down } else if (dy < 0 && !composeButton.isShown()) { - composeButton.show(); // shows it if we are scrolling up + composeButton.show(); // Shows it if we are scrolling up } } else if (!composeButton.isShown()) { composeButton.show(); @@ -358,7 +349,7 @@ public class NotificationsFragment extends SFragment implements } }; - recyclerView.addOnScrollListener(scrollListener); + binding.recyclerView.addOnScrollListener(scrollListener); eventHub.getEvents() .observeOn(AndroidSchedulers.mainThread()) @@ -382,7 +373,7 @@ public class NotificationsFragment extends SFragment implements @Override public void onRefresh() { - this.statusView.setVisibility(View.GONE); + binding.statusView.setVisibility(View.GONE); this.showingError = false; Either first = CollectionsKt.firstOrNull(this.notifications); String topId; @@ -518,7 +509,7 @@ public class NotificationsFragment extends SFragment implements @Override public void onLoadMore(int position) { - //check bounds before accessing list, + // Check bounds before accessing list, if (notifications.size() >= position && position > 0) { Notification previous = notifications.get(position - 1).asRightOrNull(); Notification next = notifications.get(position + 1).asRightOrNull(); @@ -540,7 +531,6 @@ public class NotificationsFragment extends SFragment implements @Override public void onContentCollapsedChange(boolean isCollapsed, int position) { updateViewDataAt(position, (vd) -> vd.copyWithCollapsed(isCollapsed)); - ; } private void updateStatus(String statusId, Function mapper) { @@ -615,28 +605,28 @@ public class NotificationsFragment extends SFragment implements } private void clearNotifications() { - //Cancel all ongoing requests - swipeRefreshLayout.setRefreshing(false); + // Cancel all ongoing requests + binding.swipeRefreshLayout.setRefreshing(false); resetNotificationsLoad(); - //Show friend elephant - this.statusView.setVisibility(View.VISIBLE); - this.statusView.setup(R.drawable.elephant_friend_empty, R.string.message_empty, null); + // Show friend elephant + binding.statusView.setVisibility(View.VISIBLE); + binding.statusView.setup(R.drawable.elephant_friend_empty, R.string.message_empty, null); updateFilterVisibility(); - //Update adapter + // Update adapter updateAdapter(); - //Execute clear notifications request + // Execute clear notifications request mastodonApi.clearNotifications() .observeOn(AndroidSchedulers.mainThread()) .to(autoDisposable(from(this, Lifecycle.Event.ON_DESTROY))) .subscribe( response -> { - // nothing to do + // Nothing to do }, throwable -> { - //Reload notifications on failure + // Reload notifications on failure fullyRefreshWithProgressBar(true); }); } @@ -646,10 +636,10 @@ public class NotificationsFragment extends SFragment implements bottomLoading = false; topLoading = false; - //Disable load more + // Disable load more bottomId = null; - //Clear exists notifications + // Clear exists notifications notifications.clear(); } @@ -688,7 +678,7 @@ public class NotificationsFragment extends SFragment implements window.setFocusable(true); window.setWidth(ViewGroup.LayoutParams.WRAP_CONTENT); window.setHeight(ViewGroup.LayoutParams.WRAP_CONTENT); - window.showAsDropDown(buttonFilter); + window.showAsDropDown(binding.buttonFilter); } @@ -756,12 +746,12 @@ public class NotificationsFragment extends SFragment implements } @Override - public void onViewTag(String tag) { + public void onViewTag(@NonNull String tag) { super.viewTag(tag); } @Override - public void onViewAccount(String id) { + public void onViewAccount(@NonNull String id) { super.viewAccount(id); } @@ -805,13 +795,13 @@ public class NotificationsFragment extends SFragment implements @Override public void onViewReport(String reportId) { - LinkHelper.openLink(getContext(), String.format("https://%s/admin/reports/%s", accountManager.getActiveAccount().getDomain(), reportId)); + LinkHelper.openLink(requireContext(), String.format("https://%s/admin/reports/%s", accountManager.getActiveAccount().getDomain(), reportId)); } private void onPreferenceChanged(String key) { switch (key) { case "fabHide": { - hideFab = PreferenceManager.getDefaultSharedPreferences(getContext()).getBoolean("fabHide", false); + hideFab = PreferenceManager.getDefaultSharedPreferences(requireContext()).getBoolean("fabHide", false); break; } case "mediaPreviewEnabled": { @@ -824,7 +814,7 @@ public class NotificationsFragment extends SFragment implements } case "showNotificationsFilter": { if (isAdded()) { - showNotificationsFilter = PreferenceManager.getDefaultSharedPreferences(getContext()).getBoolean("showNotificationsFilter", true); + showNotificationsFilter = PreferenceManager.getDefaultSharedPreferences(requireContext()).getBoolean("showNotificationsFilter", true); updateFilterVisibility(); fullyRefreshWithProgressBar(true); } @@ -840,7 +830,7 @@ public class NotificationsFragment extends SFragment implements } private void removeAllByAccountId(String accountId) { - // using iterator to safely remove items while iterating + // Using iterator to safely remove items while iterating Iterator> iterator = notifications.iterator(); while (iterator.hasNext()) { Either notification = iterator.next(); @@ -854,7 +844,7 @@ public class NotificationsFragment extends SFragment implements private void onLoadMore() { if (bottomId == null) { - // already loaded everything + // Already loaded everything return; } @@ -884,7 +874,7 @@ public class NotificationsFragment extends SFragment implements private void jumpToTop() { if (isAdded()) { - appBarOptions.setExpanded(true, false); + binding.appBarOptions.setExpanded(true, false); layoutManager.scrollToPosition(0); scrollListener.reset(); } @@ -892,8 +882,8 @@ public class NotificationsFragment extends SFragment implements private void sendFetchNotificationsRequest(String fromId, String uptoId, final FetchEnd fetchEnd, final int pos) { - /* If there is a fetch already ongoing, record however many fetches are requested and - * fulfill them after it's complete. */ + // If there is a fetch already ongoing, record however many fetches are requested and + // fulfill them after it's complete. if (fetchEnd == FetchEnd.TOP && topLoading) { return; } @@ -969,18 +959,18 @@ public class NotificationsFragment extends SFragment implements } if (notifications.size() == 0 && adapter.getItemCount() == 0) { - this.statusView.setVisibility(View.VISIBLE); - this.statusView.setup(R.drawable.elephant_friend_empty, R.string.message_empty, null); + binding.statusView.setVisibility(View.VISIBLE); + binding.statusView.setup(R.drawable.elephant_friend_empty, R.string.message_empty, null); } updateFilterVisibility(); - swipeRefreshLayout.setEnabled(true); - swipeRefreshLayout.setRefreshing(false); - progressBar.setVisibility(View.GONE); + binding.swipeRefreshLayout.setEnabled(true); + binding.swipeRefreshLayout.setRefreshing(false); + binding.progressBar.setVisibility(View.GONE); } private void onFetchNotificationsFailure(Throwable throwable, FetchEnd fetchEnd, int position) { - swipeRefreshLayout.setRefreshing(false); + binding.swipeRefreshLayout.setRefreshing(false); if (fetchEnd == FetchEnd.MIDDLE && !notifications.get(position).isRight()) { Placeholder placeholder = notifications.get(position).asLeft(); NotificationViewData placeholderVD = @@ -988,18 +978,18 @@ public class NotificationsFragment extends SFragment implements notifications.setPairedItem(position, placeholderVD); updateAdapter(); } else if (this.notifications.isEmpty()) { - this.statusView.setVisibility(View.VISIBLE); - swipeRefreshLayout.setEnabled(false); + binding.statusView.setVisibility(View.VISIBLE); + binding.swipeRefreshLayout.setEnabled(false); this.showingError = true; if (throwable instanceof IOException) { - this.statusView.setup(R.drawable.elephant_offline, R.string.error_network, __ -> { - this.progressBar.setVisibility(View.VISIBLE); + binding.statusView.setup(R.drawable.elephant_offline, R.string.error_network, __ -> { + binding.progressBar.setVisibility(View.VISIBLE); this.onRefresh(); return Unit.INSTANCE; }); } else { - this.statusView.setup(R.drawable.elephant_error, R.string.error_generic, __ -> { - this.progressBar.setVisibility(View.VISIBLE); + binding.statusView.setup(R.drawable.elephant_error, R.string.error_generic, __ -> { + binding.progressBar.setVisibility(View.VISIBLE); this.onRefresh(); return Unit.INSTANCE; }); @@ -1015,7 +1005,7 @@ public class NotificationsFragment extends SFragment implements bottomLoading = false; } - progressBar.setVisibility(View.GONE); + binding.progressBar.setVisibility(View.GONE); } private void saveNewestNotificationId(List notifications) { @@ -1052,8 +1042,8 @@ public class NotificationsFragment extends SFragment implements notifications.addAll(liftedNew); } else { int index = notifications.indexOf(liftedNew.get(newNotifications.size() - 1)); - for (int i = 0; i < index; i++) { - notifications.remove(0); + if (index > 0) { + notifications.subList(0, index).clear(); } int newIndex = liftedNew.indexOf(notifications.get(0)); @@ -1077,7 +1067,7 @@ public class NotificationsFragment extends SFragment implements int end = notifications.size(); List> liftedNew = liftNotificationList(newNotifications); Either last = notifications.get(end - 1); - if (last != null && liftedNew.indexOf(last) == -1) { + if (last != null && !liftedNew.contains(last)) { notifications.addAll(liftedNew); updateAdapter(); } @@ -1115,8 +1105,8 @@ public class NotificationsFragment extends SFragment implements private void fullyRefreshWithProgressBar(boolean isShow) { resetNotificationsLoad(); if (isShow) { - progressBar.setVisibility(View.VISIBLE); - statusView.setVisibility(View.GONE); + binding.progressBar.setVisibility(View.VISIBLE); + binding.statusView.setVisibility(View.GONE); } updateAdapter(); sendFetchNotificationsRequest(null, null, FetchEnd.TOP, -1); @@ -1155,7 +1145,7 @@ public class NotificationsFragment extends SFragment implements // scroll up when new items at the top are loaded while being at the start // https://github.com/tuskyapp/Tusky/pull/1905#issuecomment-677819724 if (position == 0 && context != null && adapter.getItemCount() != count) { - recyclerView.scrollBy(0, Utils.dpToPx(context, -30)); + binding.recyclerView.scrollBy(0, Utils.dpToPx(context, -30)); } } } @@ -1210,7 +1200,7 @@ public class NotificationsFragment extends SFragment implements @Override public Object getChangePayload(@NonNull NotificationViewData oldItem, @NonNull NotificationViewData newItem) { if (oldItem.deepEquals(newItem)) { - //If items are equal - update timestamp only + // If items are equal - update timestamp only return Collections.singletonList(StatusBaseViewHolder.Key.KEY_CREATED); } else // If items are different - update a whole view holder @@ -1236,7 +1226,7 @@ public class NotificationsFragment extends SFragment implements * Auto dispose observable on pause */ private void startUpdateTimestamp() { - SharedPreferences preferences = PreferenceManager.getDefaultSharedPreferences(getActivity()); + SharedPreferences preferences = PreferenceManager.getDefaultSharedPreferences(requireContext()); boolean useAbsoluteTime = preferences.getBoolean("absoluteTimeView", false); if (!useAbsoluteTime) { Observable.interval(0, 1, TimeUnit.MINUTES)