Skip to content

Conversation

j-haldane
Copy link
Contributor

@j-haldane j-haldane commented Apr 27, 2025

What is it?

  • Bugfix (user facing)
  • Feature (user facing)
  • Codebase improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

  • Supply header to LocalItemListAdapter with a supplier
  • Changes adapted from d3cd3d6

Before/After Screenshots/Screen Record

  • Before:
    Screenshot_20250427_122720
  • After:
    Screenshot_20250427_124015

Fixes the following issue(s)

  • Fixes Crash in lists (ViewHolder views not attached) #4475
  • This could be reproduced intermittently under these conditions:
    • Open the watch history list
    • Have video player drawer open and minimized
    • Scroll down far enough in the list so the header is not visible
    • Rotate the device until the bug happens
  • The header would sometimes be drawn under the list items. If a user then scrolled back up to where the header was visible this would cause a crash when the RecyclerView tried to create the attached HeaderFooterHolder.
  • Testing the above steps to reproduce the bug, I couldn't get the bug to happen again after these changes were applied.

Due diligence

@github-actions github-actions bot added the size/medium PRs with less than 250 changed lines label Apr 27, 2025
@j-haldane j-haldane changed the title Adapt header handling changes from other recyclerview adapters to fix… Fix header crash in History List view Apr 27, 2025
@AudricV AudricV added bug Issue is related to a bug GUI Issue is related to the graphical user interface labels Apr 28, 2025
@github-actions github-actions bot added size/small PRs with less than 50 changed lines and removed size/medium PRs with less than 250 changed lines labels Apr 30, 2025
@j-haldane
Copy link
Contributor Author

j-haldane commented Apr 30, 2025

After further testing I found that prior commits did fix the issue, but only because it broke an animation being applied to the header in BaseLocalListFragment. This animation was the cause of the problem. I decided to simply remove the animation instead.

This animation seems to be causing an issue with the recycling of the header. If anyone has a better solution that would allow the animation to be kept please let me know!

Personally: I can't tell the difference with or without the animation, even on a 120hz display.

Copy link

@ShareASmile ShareASmile added the history Anything to do with previously watched stuff label Apr 30, 2025
Copy link
Contributor

@Profpatsch Profpatsch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think removing this animation cannot hurt, so let’s merge (even though I can’t reproduce here, other people will have to say if that fixes it :))

@Profpatsch Profpatsch merged commit 48e826e into TeamNewPipe:dev May 6, 2025
7 checks passed
@AudricV AudricV changed the title Fix header crash in History List view Fix header crash in History list view May 7, 2025
@Stypox
Copy link
Member

Stypox commented May 7, 2025

@j-haldane thanks a lot for this, we have tried to investigate #4475 multiple times to no avail, so it's great to see it's finally getting fixed!

@j-haldane j-haldane deleted the local-list-header-fix branch May 8, 2025 22:59
@Stypox Stypox mentioned this pull request Jul 17, 2025
11 tasks
whistlingwoods pushed a commit to whistlingwoods/FoxPipe that referenced this pull request Jul 28, 2025
* Adapt header handling changes from other recyclerview adapters to fix issue TeamNewPipe#4475 in StatisticsPlaylistFragment

* Remove unneeded LayoutInflater

* Revert "Remove unneeded LayoutInflater"

This reverts commit ab73dc1.

* Revert "Adapt header handling changes from other recyclerview adapters to fix issue TeamNewPipe#4475 in StatisticsPlaylistFragment"

This reverts commit 2abe71c.

* Remove header animation causing view recycling issue
@ShareASmile
Copy link
Collaborator

ShareASmile commented Aug 9, 2025

@j-haldane would you look into it again?as it hasn't fully fixed the crash, please see the comment in the linked issue.

whistlingwoods pushed a commit to whistlingwoods/FoxPipe that referenced this pull request Aug 25, 2025
* Adapt header handling changes from other recyclerview adapters to fix issue TeamNewPipe#4475 in StatisticsPlaylistFragment

* Remove unneeded LayoutInflater

* Revert "Remove unneeded LayoutInflater"

This reverts commit ab73dc1.

* Revert "Adapt header handling changes from other recyclerview adapters to fix issue TeamNewPipe#4475 in StatisticsPlaylistFragment"

This reverts commit 2abe71c.

* Remove header animation causing view recycling issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Issue is related to a bug GUI Issue is related to the graphical user interface history Anything to do with previously watched stuff size/small PRs with less than 50 changed lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Crash in lists (ViewHolder views not attached)

5 participants