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

Remove padding from the end of the mini-player #8743

Merged

Conversation

shivambeohar
Copy link

@shivambeohar shivambeohar commented Aug 6, 2022

What is it?

  • Bugfix (user facing)

Description of the changes in your PR

  • removed right-padding of close button LinearLayout.

Before/After Screenshots/Screen Record

Before After
Before After

Fixes the following issue(s)

APK testing

The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR.

Due diligence

@shivambeohar
Copy link
Author

Hey, @mhmdanas,
Can you please review the PR, I couldn't able to add the reviewer for this PR.

@Stypox
Copy link
Member

Stypox commented Aug 6, 2022

@shivambeohar I edited the description to make it more readable, please take a look at my changes

Also see my comment in #8615 (comment)

@triallax triallax changed the title 8615-removed right-padding. Remove padding from the end of the mini-player Aug 7, 2022
@triallax triallax added feature request Issue is related to a feature in the app GUI Issue is related to the graphical user interface labels Aug 7, 2022
Copy link
Member

@Stypox Stypox left a comment

Choose a reason for hiding this comment

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

This is not the proper way to do it: tapping on R.id.overlay_buttons_layout should not close the player. What I propose is to remove all padding from the overlay_buttons_layout (you can remove the lines) and change widths, heights and paddings of the overlay_play_pause_button and overlay_close_button in order to get the same layout as the current one, but with more clickable space for those buttons.

@Stypox Stypox force-pushed the 8615-gap-at-miniplayer-close-button-fix branch from e893438 to 74a1e32 Compare November 29, 2022 11:05
@Stypox Stypox force-pushed the 8615-gap-at-miniplayer-close-button-fix branch from 74a1e32 to 2bf58ab Compare November 29, 2022 11:07
Copy link
Member

@Stypox Stypox left a comment

Choose a reason for hiding this comment

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

I pushed a commit that correctly considers the space to the right of the close button as part of the close button. I also added a content description to the buttons, and made sure the large-land layout is up-to-date with the non-large-land one. I tested on emulators, real phone and real tablet and it works.

@sonarcloud
Copy link

sonarcloud bot commented Nov 29, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@Stypox Stypox merged commit 0590350 into TeamNewPipe:dev Nov 29, 2022
@Stypox Stypox mentioned this pull request Jan 22, 2023
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issue is related to a feature in the app GUI Issue is related to the graphical user interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

There is a gap at bottom-left of miniplayer near close button
3 participants