Skip to content

Commit

Permalink
Never wrap RecyclerView with a NestedScrollView
Browse files Browse the repository at this point in the history
Wrapping a ``RecyclerView`` with a ``NestedScrollView`` breaks its recycling
functionality because the view height is stretched to fit the full list
of entries.

We never noticed performance issues in these two cases because these
lists never get very long. Let's fix these cases anyway so that we
don't accidentally base a new use of a ``RecyclerView`` on this broken
pattern.

Also renamed ``list_slots`` to ``list_groups``. Must have been
a copy-paste error.
  • Loading branch information
alexbakker committed Sep 9, 2023
1 parent dd9c307 commit ca4a3e2
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
public class GroupManagerActivity extends AegisActivity implements GroupAdapter.Listener {
private GroupAdapter _adapter;
private HashSet<String> _removedGroups;
private RecyclerView _slotsView;
private RecyclerView _groupsView;
private View _emptyStateView;
private BackPressHandler _backPressHandler;

Expand All @@ -51,11 +51,11 @@ protected void onCreate(Bundle savedInstanceState) {
}

_adapter = new GroupAdapter(this);
_slotsView= findViewById(R.id.list_slots);
_groupsView = findViewById(R.id.list_groups);
LinearLayoutManager layoutManager = new LinearLayoutManager(this);
_slotsView.setLayoutManager(layoutManager);
_slotsView.setAdapter(_adapter);
_slotsView.setNestedScrollingEnabled(false);
_groupsView.setLayoutManager(layoutManager);
_groupsView.setAdapter(_adapter);
_groupsView.setNestedScrollingEnabled(false);

for (String group : _vaultManager.getVault().getGroups()) {
_adapter.addGroup(group);
Expand Down Expand Up @@ -135,10 +135,10 @@ public boolean onOptionsItemSelected(MenuItem item) {

private void updateEmptyState() {
if (_adapter.getItemCount() > 0) {
_slotsView.setVisibility(View.VISIBLE);
_groupsView.setVisibility(View.VISIBLE);
_emptyStateView.setVisibility(View.GONE);
} else {
_slotsView.setVisibility(View.GONE);
_groupsView.setVisibility(View.GONE);
_emptyStateView.setVisibility(View.VISIBLE);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ public class IconPacksManagerFragment extends Fragment implements IconPackAdapte
@Inject
VaultManager _vaultManager;

private View _iconPacksView;
private RecyclerView _iconPacksRecyclerView;
private IconPackAdapter _adapter;
private LinearLayout _noIconPacksView;
Expand All @@ -62,7 +61,6 @@ public void onViewCreated(@NonNull View view, Bundle savedInstanceState) {

_noIconPacksView = view.findViewById(R.id.vEmptyList);
((TextView) view.findViewById(R.id.txt_no_icon_packs)).setMovementMethod(LinkMovementMethod.getInstance());
_iconPacksView = view.findViewById(R.id.view_icon_packs);
_adapter = new IconPackAdapter(this);
_iconPacksRecyclerView = view.findViewById(R.id.list_icon_packs);
LinearLayoutManager layoutManager = new LinearLayoutManager(requireContext());
Expand Down Expand Up @@ -169,10 +167,10 @@ private void startImportIconPack() {

private void updateEmptyState() {
if (_adapter.getItemCount() > 0) {
_iconPacksView.setVisibility(View.VISIBLE);
_iconPacksRecyclerView.setVisibility(View.VISIBLE);
_noIconPacksView.setVisibility(View.GONE);
} else {
_iconPacksView.setVisibility(View.GONE);
_iconPacksRecyclerView.setVisibility(View.GONE);
_noIconPacksView.setVisibility(View.VISIBLE);
}
}
Expand Down
20 changes: 4 additions & 16 deletions app/src/main/res/layout/activity_groups.xml
Original file line number Diff line number Diff line change
Expand Up @@ -18,23 +18,11 @@
android:background="?attr/colorAppBar" />
</com.google.android.material.appbar.AppBarLayout>

<androidx.core.widget.NestedScrollView
android:layout_width="match_parent"
<androidx.recyclerview.widget.RecyclerView
android:id="@+id/list_groups"
android:layout_height="match_parent"
app:layout_behavior="com.google.android.material.appbar.AppBarLayout$ScrollingViewBehavior">

<LinearLayout
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:orientation="vertical">

<androidx.recyclerview.widget.RecyclerView
android:id="@+id/list_slots"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:scrollbars="vertical"/>
</LinearLayout>
</androidx.core.widget.NestedScrollView>
android:layout_width="match_parent"
app:layout_behavior="com.google.android.material.appbar.AppBarLayout$ScrollingViewBehavior" />

<LinearLayout
android:layout_width="match_parent"
Expand Down
21 changes: 3 additions & 18 deletions app/src/main/res/layout/fragment_icon_packs.xml
Original file line number Diff line number Diff line change
Expand Up @@ -47,26 +47,11 @@

</LinearLayout>

<androidx.core.widget.NestedScrollView
android:id="@+id/view_icon_packs"
<androidx.recyclerview.widget.RecyclerView
android:id="@+id/list_icon_packs"
android:layout_width="match_parent"
android:layout_height="match_parent"
app:layout_behavior="com.google.android.material.appbar.AppBarLayout$ScrollingViewBehavior">

<LinearLayout
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:orientation="vertical">

<androidx.recyclerview.widget.RecyclerView
android:id="@+id/list_icon_packs"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:scrollbars="vertical"/>

</LinearLayout>

</androidx.core.widget.NestedScrollView>
app:layout_behavior="com.google.android.material.appbar.AppBarLayout$ScrollingViewBehavior" />

<com.google.android.material.floatingactionbutton.FloatingActionButton
android:id="@+id/fab"
Expand Down

0 comments on commit ca4a3e2

Please sign in to comment.