Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Secondary controls] Scrolling bug when opening links at the top of the description of a video #5453

Closed
4 tasks done
AudricV opened this issue Jan 18, 2021 · 15 comments · Fixed by #5946
Closed
4 tasks done
Labels
bug Issue is related to a bug GUI Issue is related to the graphical user interface

Comments

@AudricV
Copy link
Member

AudricV commented Jan 18, 2021

Checklist

Steps to reproduce the bug

  1. Make sure that description tab is the default tab before trying to reproduce the bug.
  2. Open a video with links at the top of its description.
  3. Observe app's behavior.

Actual behavior

Description is scrolling, without opening the link sometimes.

Expected behavior

Description links just open without scrolling. This bug isn't present on 0.20.9 release.

Screenshots/Screen recordings

Screencast.Links.description.secondary.controls.mp4

If the description tab is the default tab before opening a content (after an app start), the description view scrolls a little bit, like in this screen recording:

Screencast.Secondary.controls.second.bug.mp4

Device info

  • Android version/Custom ROM version: EMUI 10 / Android 10
  • Device model: Honor 9X
@AudricV AudricV added the bug Issue is related to a bug label Jan 18, 2021
@AudricV AudricV mentioned this issue Jan 19, 2021
14 tasks
@Stypox
Copy link
Member

Stypox commented Jan 19, 2021

Does this apk from ci in #5460 fix the issue?

@AudricV
Copy link
Member Author

AudricV commented Jan 19, 2021

@Stypox Unfortunately, no. See screencast below attached as a ZIP file:

No fix fix-scroll-description apk.mp4.zip

@Stypox
Copy link
Member

Stypox commented Jan 20, 2021

I don't know how to fix this. Android seems to like moving views around when giving focus to them (even though they are already visible), and I don't know how we could prevent that. @vkay94 do you have any idea how this could be solved? I found this SO but wasn't able to find a good solution (i.e. I could disable automatic scrolling when tapping on links but then selecting would not work)

@vkay94
Copy link
Contributor

vkay94 commented Jan 20, 2021

@Stypox I'll take a look into it (I have to look into the implementation first)

Edit: My investigation results in the following (based on the dev branch):

Just to make sure: the behavior that the description tab scrolls on focus request (tapping somewhere, not necessarily on links) is intended?

The reason for this scrolling is the recreation of the DescriptionFragment creation which cause relies on the viewpager (the default offset is 1) - if you switch between related videos and description the bug doesn't occur.

So it is enough to increase the value to 2: binding.viewPager.setOffscreenPageLimit(2)

But there is a small other bug then:
Once the description TextView gains focus and you switch to another tab - while the "Released on xyz" is visible - and back again to the description, the NestedScrollView is a little bit offset, so that only the description is visible.

@Stypox
Copy link
Member

Stypox commented Jan 20, 2021

@vkay94 this also happens when the description is the first tab that shows up when opening a video

@vkay94
Copy link
Contributor

vkay94 commented Jan 20, 2021

Perfect timing :') So it isn't intentional (I was focussing on the tab switch). To fix it (scrolling on tap) you just have to add clickable="false" to TextView with id = "detail_description_view" in fragment_details layout (the small offset scroll on focus/tap remains as described at the bottom of the previous comment).

Edit: Or not ... let me see again. I'll ping you when I find a workaround for both xD

@Stypox
Copy link
Member

Stypox commented Jan 23, 2021

Does this other apk from ci in #5460 fix the issue?
@TiA4f8R btw, I replied you on IRC

@AudricV
Copy link
Member Author

AudricV commented Jan 23, 2021

@Stypox No, it doesn't but there is a change: when clicking the description tab, the description view doesn't scroll to the top of the screen. However, clicking on a link scrolls the description view to top of the screen.

@Stypox
Copy link
Member

Stypox commented Jan 23, 2021

@TiA4f8R then I can't reproduce

@Stypox
Copy link
Member

Stypox commented Jan 23, 2021

@TiA4f8R I pushed another commit to #5460, could you test it when it gets built? I tried with something else

@AudricV
Copy link
Member Author

AudricV commented Jan 23, 2021

@Stypox Same result as the previous :(

@AudricV
Copy link
Member Author

AudricV commented Feb 15, 2021

@Stypox @vkay94 With 0.20.10, this bug is now more strange. It just scroll a bit if the description tab is the default tab before opening a content for the first time you open an instance of the app. See the second screencast in the top comment.

@AudricV AudricV added the GUI Issue is related to the graphical user interface label Mar 4, 2021
@Stypox
Copy link
Member

Stypox commented Mar 31, 2021

Ok, so I tried to investigate this a little bit more. It seems like this is not a bug in NewPipe's code, but in how things are implemented in Android. When text in a TextView is made selectable, and the TextView is in a layout where the app bar (i.e. the player, in NewPipe's case) collapses when scrolling, things start behaving strangely. Actually, I tried another simple test app, which uses a similar scrolling behaviour to NewPipe's, and as soon as I add isTextSelectable="true" here the flickering behavior appears. I doubt there is any better way to solve this other than making the text not selectable and then think of another way to let the user copy the text.

@Stypox
Copy link
Member

Stypox commented Mar 31, 2021

@TiA4f8R Could you confirm that this APK fixes the scrolling bug (even though it does not allow selecting, and tapping links does not show the white background underneath)?
app-debug.zip

@AudricV
Copy link
Member Author

AudricV commented Mar 31, 2021

@Stypox Yes

Stypox added a commit to Stypox/NewPipe that referenced this issue Apr 1, 2021
Since now selection is disabled by default, this fixes TeamNewPipe#5453
Stypox added a commit to Stypox/NewPipe that referenced this issue Apr 1, 2021
Since now selection is disabled by default, this fixes TeamNewPipe#5453
Stypox added a commit to Stypox/NewPipe that referenced this issue Apr 1, 2021
Since now selection is disabled by default, this fixes TeamNewPipe#5453
Stypox added a commit to Stypox/NewPipe that referenced this issue Jun 2, 2021
Since now selection is disabled by default, this fixes TeamNewPipe#5453
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
Projects
None yet
3 participants