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

Playlist page improvements #2281

Merged

Conversation

vallode
Copy link
Contributor

@vallode vallode commented May 31, 2022


Playlist page improvements

Pull Request Type
Please select what type of pull request this is:

  • Bugfix

Related issue
Closes #2255

Description
Small love and care to the playlist templates:

  • Add thumbnail hover visual on videos
  • Fixes issue with overflow not being visible on the playlist description
  • Fixes issue where playlist description whitespaces would not be rendered
  • Changes the channel icon to be a bit smaller to save space
  • Changes a few styles to the playlist stats

Screenshots (if appropriate)
Please add before and after screenshots if there is a visible change.

Before:
image

After:
image

Testing (for code that is not small enough to be easily understandable)
Just played around with playlists and smaller views.

Desktop (please complete the following information):

  • OS: [e.g. iOS] 10
  • OS Version: [e.g. 22] Debian
  • FreeTube version: [e.g. 0.8] upstream/development

@PrestonN PrestonN enabled auto-merge (squash) May 31, 2022 16:59
@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc added the PR: waiting for review For PRs that are complete, tested, and ready for review label May 31, 2022
Copy link
Member

@absidue absidue left a comment

Choose a reason for hiding this comment

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

Please fix the missing new lines at the end of the files and the share menu getting cut off.

Playlist in the screenshots: https://www.youtube.com/playlist?list=PLDIoUOhQQPlXr63I_vwF9GD8sAKh77dWU

Share menu in development:
development-share

Normal with this PR:
this-pr-normal

Hovering over the share menu with this PR:
this-pr-share

src/renderer/components/ft-list-video/ft-list-video.sass Outdated Show resolved Hide resolved
src/renderer/views/Playlist/Playlist.css Outdated Show resolved Hide resolved
@absidue absidue added PR: changes requested and removed PR: waiting for review For PRs that are complete, tested, and ready for review labels May 31, 2022
auto-merge was automatically disabled June 1, 2022 12:27

Head branch was pushed to by a user without write access

@PrestonN PrestonN enabled auto-merge (squash) June 1, 2022 12:27
@vallode
Copy link
Contributor Author

vallode commented Jun 1, 2022

@absidue yeahhhh, you can scroll and see them but it's not ideal agreed. Going to need a hot second to figure out a good solution. EDIT: I've pushed a bandaid fix that I think is fine for now but will be removed once I get around to cleaning up some of the main layout styling in the app (the margins everywhere make positioning and sizing a nightmare)

auto-merge was automatically disabled June 1, 2022 12:59

Head branch was pushed to by a user without write access

@PrestonN PrestonN enabled auto-merge (squash) June 1, 2022 12:59
auto-merge was automatically disabled June 1, 2022 13:01

Head branch was pushed to by a user without write access

@PrestonN PrestonN enabled auto-merge (squash) June 1, 2022 13:01
@efb4f5ff-1298-471a-8973-3d47447115dc

styling has been a problem for a long time see #856 :(

@absidue absidue added PR: waiting for review For PRs that are complete, tested, and ready for review and removed PR: changes requested labels Jun 2, 2022
absidue
absidue previously approved these changes Jun 2, 2022
Copy link
Member

@absidue absidue left a comment

Choose a reason for hiding this comment

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

Looks good now. Thanks.

auto-merge was automatically disabled June 3, 2022 13:21

Head branch was pushed to by a user without write access

@PrestonN PrestonN enabled auto-merge (squash) June 3, 2022 13:21
auto-merge was automatically disabled June 3, 2022 13:21

Head branch was pushed to by a user without write access

@PrestonN PrestonN enabled auto-merge (squash) June 3, 2022 13:22
Copy link
Member

Choose a reason for hiding this comment

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

LGTM!

@PrestonN PrestonN merged commit fd46b22 into FreeTubeApp:development Jun 4, 2022
@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Jun 4, 2022
@vallode vallode deleted the feature/playlist-improvements branch June 4, 2022 23:37
MarmadileManteater added a commit to MarmadileManteater/FreeTubeAndroid that referenced this pull request Feb 14, 2024
Originally, when this overflow-y was added, the share playlist dropdown went straight down, so there would no need for the content to be able to overflow in the y direction. (FreeTubeApp#2281)

Now, the dropdown swings out to the left by default, and even when it is centered, it still overflows the width of the container as an intentional part of the design.
FreeTubeBot pushed a commit that referenced this pull request Feb 15, 2024
* Set share playlist dropdown position to `center` w/ media query

* Remove overflow-y as it causes the overflow issue

Originally, when this overflow-y was added, the share playlist dropdown went straight down, so there would no need for the content to be able to overflow in the y direction. (#2281)

Now, the dropdown swings out to the left by default, and even when it is centered, it still overflows the width of the container as an intentional part of the design.

* Remove unnecessary whitespace
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.

[Bug]: Some elements in the dropdown on the playlist page are inaccessible
6 participants