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

issue #2670 - Feature to remove thumbnail dots #2678

Merged
merged 4 commits into from
Nov 17, 2024

Conversation

marvinl803
Copy link
Contributor

@marvinl803 marvinl803 commented Nov 14, 2024

Summary:

This pull request works on issue #2670 removes the dots from the thumbnails.

Please review and let me know if any additional changes are required.

Before:
issue-1

After:
issue-2

@ImprovedTube
Copy link
Member

ImprovedTube commented Nov 15, 2024

thank you again!

  • for the name?: " ⫶ " (more actions)

  • why did you use JS here unlike in the other PR?

    • then we can also make it depend on ImprovedTube.storage ... (Our CSS variables are convenient, but conditional JS can be more CPU efficient, since not everybody will use the feature and CSS rules may be evaluated /processed from right to left.)

@marvinl803
Copy link
Contributor Author

Hi,

I have made the requested changes. Initially, I attempted to implement the solution the same way I did in my previous PR, but it wasn't working as expected. After investigating, I found the issue and have now implemented the feature in the same way as my last PR. It should work correctly now.

Additionally, I updated the name as requested. Please let me know if it's correct or if you would like me to make further adjustments.

Thank you!

@HornyPrivateGamer
Copy link

Summary:

This pull request works on issue #2670 removes the dots from the thumbnails.

Please review and let me know if any additional changes are required.

Before: issue-1

After: issue-2

Hi any way you could remove those margins. That there is a big empy space after removing the three dots.
Thansk

@ImprovedTube ImprovedTube merged commit c78dcd4 into code-charity:master Nov 17, 2024
1 check passed
@ImprovedTube
Copy link
Member

have now implemented the feature in the same way as my last PR. It should work correctly now.

thanks

we can also make it depend on ImprovedTube.storage ... (Our CSS variables are convenient, but conditional JS can be more CPU efficient, since not everybody will use the feature and CSS rules may be evaluated /processed from right to left.)

, while ultimately we can reduce CSS load by moving most CSS to JS, depending if (ImprovedTube.storage...)'s as mentioned (didn't benchmark exactly) ( but that's beyond #2678 )

@marvinl803 marvinl803 deleted the feature-issue-2670 branch November 17, 2024 19:54
@marvinl803
Copy link
Contributor Author

@HornyPrivateGamer I will look to see if I can come up with a solution for those whitespaces and I will make a new PR with those changes.

@HornyPrivateGamer
Copy link

@HornyPrivateGamer I will look to see if I can come up with a solution for those whitespaces and I will make a new PR with those changes.

ok thanks a lot, you the best

@ImprovedTube
Copy link
Member

, while ultimately we can reduce CSS load by moving most CSS to JS, depending if (ImprovedTube.storage...)'s as mentioned (didn't benchmark exactly) ( but that's beyond #2678 )

...or, in other words, next to our layout features, we can make a collection of all relevant uBlock origin rules (1000's which need no update or are updated by their authors or have a million users all updating manually yet), so then the main work remaining about this will be naming / explaining each optionally and maybe crowd-sourcing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants