Make "Playback speed controls" dialog contents more compact #6150
Conversation
|
Thank you. Can you post screenshots of the changes? And please give your PR a proper title |
|
@AmooHashem Can you do what @TobiGr said above, please? Thank you! |
triallax
changed the title
|
Sorry for late answering. |
triallax
added
feature request
|
Instead of a scrollable window pane which is very confusing, it would be much more intuitive to have a 2 column grid with extra speed steps, i.e. 1.75x 2x 2.5x and 3x The word Speed needs to look like a button or nobody will know it can be changed. Reset, OK and cancel buttons are too close together and could be accidentally pressed. |
|
Why not just making it so that the speed button in the popup open the same dialog as the background and normal players? |
| android:paddingTop="@dimen/video_item_search_padding" | ||
| android:paddingRight="@dimen/video_item_search_padding"> | ||
|
|
||
| <RelativeLayout |
There was a problem hiding this comment.
While you are at it, please convert everything to a single ConstraintLayout nested inside a ScrollView, it will make everything simpler and with less nesting.
| @@ -14,6 +14,7 @@ | |||
| <dimen name="video_item_search_thumbnail_image_width">142dp</dimen> | |||
| <dimen name="video_item_search_thumbnail_image_height">80dp</dimen> | |||
| <!-- Calculated: 2*video_item_search_padding + video_item_search_thumbnail_image_height --> | |||
| <dimen name="video_item_search_padding">18dp</dimen> | |||
There was a problem hiding this comment.
What is this? Also, pay attention to the comment in the line above (16): I think it refers to the line below (18), not to this one (15)
|
@SameenAhnaf I don't think your menu is a good idea. Designing small unfamiliar menus with not-so-clear actions just for the purpose of reducing used space is not a good option. The current menu is ok, there's no need to reinvent the wheel, otherwise the app will become unusable for everyone except experts and very difficult to maintain due to the many bugs that would arise. When you propose a solution, please think about this: is it a standard component or would it require designing a completely new layout with strange actions? is it easily understandable by someone who has never ever used the app? are the various actions easy to click on (i.e. no small buttons), even on Android TV (i.e. navigating with ←↑↓→ is obvious)? |
This comment has been minimized.
This comment has been minimized.
|
A scrollable menu in this usage case is terrible ux. It took me minutes to figure out what you meant and that's with you explaining and pointing it out. Casual users have no hope. Also for me it would take more button pushes than the existing menu. |
|
"Unhook" and "FF" are not meaningful to a new user. Changing pitch can only be done by accessing yet another nested menu. The graphic would be difficult to implement and would not provide any real advantage.
I sometimes use them, someone who wants to play guitar a little bit slower (e.g. 8%) would use them, ... Why not just keep the current layout and call it a day? I mean, it provides all of the needed features. This PR fixes the problem arising on tablets/landscape, when the user would have to scroll down in order to be able to reach all of the buttons, and that definitely needs fixing. But let's not reinvent the wheel just because a button may be more accessible... that can usually be solved in simpler ways, e.g. just by moving things around on tablets/landscape like this PR did. |
There was a problem hiding this comment.
(This comment applies only to the landscape layout) I just tested and noticed the "Fast-forward during silence" button overlays the "Unhook" text. This has to be solved. I would also move the "Tempo" and "Pitch" labels to the left of the seekbars, as there is plenty of horizontal space, and remove as much vertical spacing as possible (but it should still look good, not cluttered).
|
What about making "presets" for speed playback? Without having to modify it every time, buttons with common speeds would be interesting (1.25x, 1.50x, 2x) To explain better I use a lot AntennaPod and I like this speed options a lot |
|
Superseded by #7363. Anyway thank you for the PR 😄 |
What is it?
Description of the changes in your PR
Fixes the following issue(s)
Relies on the following changes
APK testing
On the website the APK can be found by going to the "Checks" tab below the title and then on "artifacts" on the right.
Due diligence