search

ReVanced / revanced-patches

🧩 Patches for ReVanced
https://revanced.app
GNU General Public License v3.0
2.44k stars 284 forks source link

bug(YouTube): Chapter title overlaps with download and copy url buttons #2178

Closed HeroGalak closed 1 month ago

HeroGalak commented 2 years ago

Type

Cosmetic

Bug description

The title is self explanatory

Steps to reproduce

Build YT 17.29.34 with latest patches, CLI and integrations.

Relevant log output

//

Screenshots or videos

Screenshot of the issue: Screenshot_20220910-152654_YouTube

Solution

Possible solution: https://github.com/orgs/revanced/discussions/558

(This solution would solve also the conflict with VR button )

Additional context

No response

oSumAtrIX commented 2 years ago

Another solution would be to decrease the width of the chapters text, when the download button is visible.

HeroGalak commented 2 years ago

Another solution would be to decrease the width of the chapters text, when the download button is visible.

But if you decrease the width of the text you would cut the chapter's title. Moreover the conflict with VR button would still remain.

oSumAtrIX commented 2 years ago

The chapters title is still being cut if its too long anyways.

SodaWithoutSparkles commented 2 years ago

Another solution would be implement ReVanced/revanced-patches-template#1958 and remove the need for a download button inside the player

ILoveOpenSourceApplications commented 1 year ago

Does this issue still exist?

SodaWithoutSparkles commented 1 year ago

Dun know. Any link to a sample video that has the issue?

ILoveOpenSourceApplications commented 1 year ago

https://youtu.be/LRhMsaa9R0I

SodaWithoutSparkles commented 1 year ago

Screenshot_2023-02-15-00-57-38-556_app.revanced.android.youtube-edit.jpg

Still an issue.

Edit: I wonder what would happen if there is a XXXL title that makes Revanced or YT face the same issue in landscape mode as well.

ILoveOpenSourceApplications commented 1 year ago

Still an issue.

Edit: I wonder what would happen if there is a XXXL title that makes Revanced or YT face the same issue in landscape mode as well.

Maybe or maybe not. Will have to find a video having a really long chapter title.

ILoveOpenSourceApplications commented 1 year ago

Given that this bug depends to the chapter title of the videos, i.e, longer the title, the more the overlay buttons will come in contact with the chapter title. So can this solved?

Possible solutions would be:

  1. Resize the entire button layout to be small, but that would create issues with the full-screen button or may not even be a solution if an even longer title shows up.
  2. Disable chapter title within videos which I believe is already implemented and if not a decent solution.
  3. Open to ideas 🤷🏻‍♂️.
SodaWithoutSparkles commented 1 year ago

Resize the chapter title box and truncate the overflow ones?

LisoUseInAIKyrios commented 1 year ago

Or maybe move the save video and copy video url buttons to the same area as the sponsorblock buttons? It looks like there is just enough room

oSumAtrIX commented 1 year ago

That room would be preserved for the YouTube video title.

SodaWithoutSparkles commented 1 year ago

Or maybe move the save video and copy video url buttons to the same area as the sponsorblock buttons? It looks like there is just enough room

On that note, I don't think both copy url & copy url w/ time should be enabled by default at the same time.

I'd prefer just having the one w/ time, as it would be less cluttered. If you don't want the timestamp, just press backspace a few times or use the default one.

Should I open a new FR? Or bug report? Or PR myself? 🤔

oSumAtrIX commented 1 year ago

I guess just leaving the button for copying with timestamp by default works.

LisoUseInAIKyrios commented 1 year ago

Maybe it's possible to change the url that's passed to the android share sheet, and add the timestamp to the share url? That removes the need for extra buttons, since the share sheet already has a copy url option.

oSumAtrIX commented 1 year ago

People wanted an extra button. For the same reason they wanted a download button when they were able to utilize the share sheet to begin with.

Supebuff commented 1 year ago

Screenshot_20230301_233600_Chrome~3 Why not add a button to accommodate more buttons?

SodaWithoutSparkles commented 1 year ago

Why not add a button to accommodate more buttons?

Great idea. This way any future buttons would only need to be added inside the sub-menu. Let's call it the tools menu for now. This way, ensuring compatibility between patches (overlapping) would be much easier.

We could move the tools button above the full screen button, so it won't ever overlap with youtube's new buttons (watch in VR ReVanced/revanced-patches-template#2220, audio tracks ReVanced/revanced-patches-template#1092, etc).

Maybe like a wrench icon? Tho it must suit the current design language of buttons, similar to the downloads & copy video timestamps

Screenshot_2023-04-29-10-43-50-984_app.revanced.android.youtube-edit.jpg

ignore my poor drawing skills

trmdi commented 1 year ago

Why not just replace the full screen button with the tool menu button in case we have > 1 button there. Since we can swipe up to go into the full screen mode, that button is not really needed, we can safely put it into the tool button.

Domiiniik commented 1 year ago

This is pretty old bug and to this day nothing has been tried to do something about it, hopefully waiting for a fix sooner or later. ❤️✊️ ReVanced ftw