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

Exoplayer: Increase space between rewind, pause, ffwd buttons #4077

Merged
merged 1 commit into from
Nov 1, 2023

Conversation

mcclure
Copy link
Collaborator

@mcclure mcclure commented Oct 25, 2023

I [posted our new video player layout] on Mastodon for comments and multiple people said the buttons were too close together. I agree. I added some space (I eyeballed it, I made it bigger until it felt too big and then I narrowed it), I think we have now increased the space from 10dp to 25dp. I added the space by wrapping the buttons in LinearLayouts, because they are s and could theoretically insert more than one button.

Concerns: If the "next"/"prev" buttons ever become active, the space will not be correctly applied to those. We can fix that if it ever comes up (we don't display those buttons). If people think the buttons should be placed even further apart we can do this by just increasing the number in styles.xml.

This is what it looks like now. See previous look and comparison with 23.0 in #4071

@connyduck
Copy link
Collaborator

I don't think the additional wrapper is necessary. You could instead add margin to the middle button, or override the styles used by the buttons, ExoStyledControls.Button.Center.RewWithAmount and ExoStyledControls.Button.Center.FfwdWithAmount.

@mcclure
Copy link
Collaborator Author

mcclure commented Oct 31, 2023

Will attempt to implement Conny's suggestion today, we should merge this only after that.

@mcclure
Copy link
Collaborator Author

mcclure commented Oct 31, 2023

New patch should follow Conny's requests. Looks almost identical to above, the buttons seem to have widened by like 1px. (Moving 20dp down to 19dp made it narrow by 1px, so I decided to leave it at 20dp.)

@Tak Tak merged commit ede66c4 into tuskyapp:develop Nov 1, 2023
3 checks passed
@charlag charlag added this to the Tusky 24 milestone Nov 18, 2023
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.

4 participants