Fast scrolling - loosing focus when having images from an url in the items

fankloano commented 8 months ago

Hello again,

another issue I had some time ago: Having a vertical recyclerview with an imageview inside the item (loading the image from an url to the imageview), when I was scrolling fast I always lost the focus (mostly when I was getting to the bottom or top of the list).

I was able to solve it using: setSmoothFocusChangesEnabled(false)

So I don't know if it's really a bug (maybe it has also to do with the image libraries I tested [Glide & Picasso]), but eventually you want to have a look at it. (for me it's not mandatory, as there is a work-around).

rubensousa commented 8 months ago

Which views are you marking as focusable? If it's the root view of your ViewHolder, focus should persist there. A sample here would also help

fankloano commented 8 months ago

I have following recyclerview:

<com.rubensousa.dpadrecyclerview.DpadRecyclerView
        android:id="@+id/rv_layout_TvChannels"
        android:layout_width="0dp"
        android:layout_height="match_parent"
        android:orientation="vertical"
        app:layout_constraintStart_toEndOf="@id/linLayout_tv_categories"
        app:layout_constraintTop_toTopOf="parent"
        app:layout_constraintBottom_toBottomOf="parent"
        app:layout_constraintWidth_percent="0.50"
        tools:listitem="@layout/rv_item_tvchannels"/>

and the root view of the rv_item_tvchannels holds the focus:

<?xml version="1.0" encoding="utf-8"?>
<androidx.constraintlayout.widget.ConstraintLayout xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:tools="http://schemas.android.com/tools"
    android:id="@+id/cardView_tvchannel"
    android:layout_width="match_parent"
    android:layout_height="80dp"
    android:focusable="true"
    android:focusableInTouchMode="true"
    android:descendantFocusability="afterDescendants"
    android:background="@drawable/channel_item_background"
    xmlns:app="http://schemas.android.com/apk/res-auto">

other views....

So in the adapter to handle key events etc. I am using always cardView_tvchannel

rubensousa commented 8 months ago

You shouldn't use android:descendantFocusability="afterDescendants if the focus should stay in that view. Please remove that or replace it with beforeDescendants

rubensousa commented 8 months ago

You can also figure out where the focus is being held with the developer option "Show layout bounds". This should show you a cross on the view that's receiving focus. You can also use these APIs to see if they solve your problem: https://rubensousa.github.io/DpadRecyclerView/api/dpadrecyclerview/com.rubensousa.dpadrecyclerview/-dpad-recycler-view/set-focus-out-allowed.html

fankloano commented 8 months ago

Changing android:descendantFocusability="afterDescendants to beforeDescendants or removing it didn't help. Using the focus out allowed apis neither (I was already using them):

private fun prepareTvChannelsRecyclerView() {
        tvChannelsAdapter = TvChannelsAdapter(onChannelClickListener, onChannelLongClickListener,this, helpViewModel)
        binding.rvLayoutTvChannels.apply {
            adapter = tvChannelsAdapter
            addItemDecoration(
                DpadLinearSpacingDecoration.create(
                    itemSpacing = 6,
                    edgeSpacing = 6,
                    perpendicularEdgeSpacing = 6
                )
            )
            setFocusOutAllowed(false, false)
            setFocusOutSideAllowed(false, false)
        }
    }

The only thing that helped was setting: setSmoothFocusChangesEnabled(false), with this the issue is gone. Why could it work with set to false?

I think I know where the focus goes (when loosing the focus, navigating up or down changes the channels, as I would be in the tv_categories_recyclerview):

<?xml version="1.0" encoding="utf-8"?>
<androidx.constraintlayout.widget.ConstraintLayout xmlns:android="http://schemas.android.com/apk/res/android"
    android:layout_width="match_parent"
    android:layout_height="match_parent"
    android:id="@+id/const_tv"
    android:background="?attr/backgroundLight"
    android:descendantFocusability="afterDescendants"
    xmlns:app="http://schemas.android.com/apk/res-auto"
    xmlns:tools="http://schemas.android.com/tools">

    <LinearLayout
        android:id="@+id/linLayout_tv_categories"
        android:layout_width="0dp"
        android:layout_height="match_parent"
        android:orientation="vertical"
        android:background="?attr/backgroundLight"
        app:layout_constraintStart_toEndOf="@id/linLayout_tvCountries_menu"
        app:layout_constraintTop_toTopOf="parent"
        app:layout_constraintWidth_percent="0.20">

        <TextView
            android:id="@+id/tv_focusedCountry"
            android:layout_width="match_parent"
            android:layout_height="36dp"
            android:gravity="center_vertical"
            android:paddingStart="18dp"
            android:paddingEnd="10dp"
            android:layout_margin="1dp"
            android:background="?attr/backgroundMain"
            android:textColor="?attr/textMainColor"
            android:textSize="12sp"/>

    <com.rubensousa.dpadrecyclerview.DpadRecyclerView
        android:id="@+id/rv_layout_tv_categories"
        android:layout_width="match_parent"
        android:layout_height="wrap_content"
        android:background="#34373C"
        android:orientation="vertical"
        android:layout_marginStart="2dp"
        android:layout_margin="1dp"
        tools:listitem="@layout/rv_item_tvcategory"
        app:layout_constraintStart_toEndOf="@id/rv_layout_tvCountries_menu"
        app:layout_constraintTop_toTopOf="parent"
        app:layout_constraintWidth_percent="0.20"/>

    </LinearLayout>

    <LinearLayout
        android:id="@+id/linLayout_tvCountries_menu"
        android:layout_width="0dp"
        android:layout_height="0dp"
        app:layout_constraintTop_toTopOf="parent"
        tools:listitem="@layout/rv_item_activated_countries"
        app:layout_constraintStart_toStartOf="parent"
        app:layout_constraintBottom_toBottomOf="parent"
        android:orientation="vertical"
        app:layout_constraintWidth_percent="0.20">

        <View
            android:layout_width="match_parent"
            android:layout_height="36dp"
            android:layout_margin="1dp"
            android:background="?attr/backgroundMain"/>

    <com.rubensousa.dpadrecyclerview.DpadRecyclerView
        android:id="@+id/rv_layout_tvCountries_menu"
        android:layout_width="match_parent"
        android:layout_height="wrap_content"
        android:background="?attr/backgroundMain"
        android:orientation="vertical"
        android:layout_margin="1dp"
        app:layout_constraintTop_toTopOf="parent"
        tools:listitem="@layout/rv_item_activated_countries"
        app:layout_constraintStart_toStartOf="parent"
        app:layout_constraintWidth_percent="0.20"/>

    </LinearLayout>

    <com.rubensousa.dpadrecyclerview.DpadRecyclerView
        android:id="@+id/rv_layout_TvChannels"
        android:layout_width="0dp"
        android:layout_height="match_parent"
        android:orientation="vertical"
        app:layout_constraintStart_toEndOf="@id/linLayout_tv_categories"
        app:layout_constraintTop_toTopOf="parent"
        app:layout_constraintBottom_toBottomOf="parent"
        app:layout_constraintWidth_percent="0.50"
        tools:listitem="@layout/rv_item_tvchannels"/>

rubensousa commented 8 months ago

If this still didn't work, I would appreciate a drawing with the problematic focus states please. Show me where the focus is held when you fast scroll. setSmoothFocusChangesEnabled(false) will trigger layout requests for each position change, which is more expensive. Maybe you could modify the sample project on this repo to reproduce this issue so I can have a look at your fork

fankloano commented 8 months ago

@rubensousa I made a video of loosing the focus, or what do you mean by drawing? https://github.com/rubensousa/DpadRecyclerView/assets/83220273/fc627d25-17f7-4f9e-a9f5-5fddbebefd94

So I have to fork the sample project, modify it in android studio and update the fork with it, is this correct? (sorry, never done this before :-) )

rubensousa commented 8 months ago

Looks like you're updating the adapter while fast scrolling. Are you using DiffUtil to apply diffing or requesting focus somewhere else? Please enable the "Show layout bounds" and then upload a new recording.

If you can, please fork the project and create a sample inside it that reproduces the issue.

fankloano commented 8 months ago

Yes i am using DiffUtil in the adapter:

  companion object {
        private val MANAGE_TVCHANNELS_COMPERATOR = object : DiffUtil.ItemCallback<TvChannelsWithEpgData>() {
            override fun areItemsTheSame(oldItem: TvChannelsWithEpgData, newItem: TvChannelsWithEpgData) =
                oldItem.tvChannel.channelId == newItem.tvChannel.channelId

            @SuppressLint("DiffUtilEquals")
            override fun areContentsTheSame(oldItem: TvChannelsWithEpgData, newItem: TvChannelsWithEpgData) =
                oldItem.epgDataList == newItem.epgDataList &&
                        oldItem.tvChannel.showingName == newItem.tvChannel.showingName &&
                        oldItem.tvChannel.epgChannelId == newItem.tvChannel.epgChannelId &&

        }
    }

and notifyitemchanged in the fragment to update specific channels (I just removed the notifyitemchanged parts, but it didn't solve the problem). Following the video with enabled Show layout bounds..

https://github.com/rubensousa/DpadRecyclerView/assets/83220273/4c4a4c43-35e2-48f8-b5d4-a7237677e873

ps. For forking the project etc. I need some time, as I never did that before and I a have only short time on PC the rest of this + most of the next week (easter, family, kids..)

fankloano commented 7 months ago

@rubensousa Just to let you know, I've been sick in bed for a few days (not better yet) . If you're still waiting for the fork, it may take a while, sorry...

fankloano commented 6 months ago

@rubensousa Sorry, but it seems that I am to stupid to fork and modify.. Do I have to add also things like RetrofitInstance etc.? So everything I need or use in my app?? Or would there be an easier way, so that you can investigate the issue? (eventually step by step what happens in the issue-part of my app?)

Thanks, Alex

rubensousa commented 6 months ago

@fankloano No, you don't. Just create a similar project in github that reproduces the problem so I can investigate. You don't need to copy your real app

homayoonahmadi commented 6 months ago

Same here, I think this happens because of lazy updating of the list that changes positions in the list, and when I disable smooth focus change, it doesn't happen anymore

rubensousa commented 6 months ago

@homayoonahmadi can you share a sample that reproduces this problem?