Some layout optimizations

[maniac103]

While Squeezer works great, I found the look'n'feel has some optimization potential ;-) This PR contains a few assorted optimizations that don't change the overall app appearance too much, but are intended to just add some polish. For details see individual commits.

Please let me know what you think about those; if you're OK with the general direction I'd go on in that direction.

[maniac103]

Is there any particular reason you prefer LinearLayout over ContraintLayout?

The effect I want to achieve is the combination of text1 and text2 being centered vertically in the parent view (if they're smaller than the artwork image), but at the same time the image to be vertically centered if both texts in combination are taller than the image ... and I'm not sure how to achieve that with ConstraintLayout. With your suggestion, won't both texts essentially be bottom-aligned?

This new implementation controls text2 visibility per item. I think it looks fine, but I don't know about design guidelines, and I'm curious to what you think?

Well, while I think that argument is technically correct, I'm not sure whether there's any actual difference in practice: all lists that I have seen so far have either text2 populated or haven't, I haven't seen a mixed case so far. In any case, personally I much prefer slight potential visual inconsistency over having all items look unaligned (since text2 being unpopulated is the vast majority of list items).

This obsoletes VIEW_PARAM_TWO_LINE (which exists for historical reasons).

I will have a look into that.

Keyboard doesn't hide

I'll double check this. It worked in the API34 emulator, but indeed not on my API33 device.

We need dividers for the final list (not necessary for grouped results)

TBH, my proposal would be removing dividers entirely, maybe replacing them with a bit more padding ;-)

[maniac103]

This obsoletes VIEW_PARAM_TWO_LINE (which exists for historical reasons).

I've integrated the optional hiding of the text2 line into the VIEW_PARAM_TWO_LINE framework.

Keyboard doesn't hide

Should be fixed now, by focussing the button before attempting to hide the keyboard.

[kaaholst]

Is there any particular reason you prefer LinearLayout over ContraintLayout?

The effect I want to achieve is the combination of text1 and text2 being centered vertically in the parent view (if they're smaller than the artwork image), but at the same time the image to be vertically centered if both texts in combination are taller than the image ... and I'm not sure how to achieve that with ConstraintLayout. With your suggestion, won't both texts essentially be bottom-aligned?

This new implementation controls text2 visibility per item. I think it looks fine, but I don't know about design guidelines, and I'm curious to what you think?

Well, while I think that argument is technically correct, I'm not sure whether there's any actual difference in practice: all lists that I have seen so far have either text2 populated or haven't, I haven't seen a mixed case so far. In any case, personally I much prefer slight potential visual inconsistency over having all items look unaligned (since text2 being unpopulated is the vast majority of list items).

The effect can be seen by adding a shortcut tom the Home screen, like in the screenshot below. I also prefer this look. To answer the question above, this is achieved by adding the 2 lines to the ConstraintLayout XML. Link to documentation. I believe Spreadwill give the desired result. An app:layout_constraintBottom_toBottomOf="parent" also have to be added to cover art and context menu button to vertically center those, however.

This obsoletes VIEW_PARAM_TWO_LINE (which exists for historical reasons).

I will have a look into that.

Your new commit works fine, that was quick!

Keyboard doesn't hide

I'll double check this. It worked in the API34 emulator, but indeed not on my API33 device. Your new commit works fine!

We need dividers for the final list (not necessary for grouped results)

TBH, my proposal would be removing dividers entirely, maybe replacing them with a bit more padding ;-)

This screenshot shows the missing dividers (and also shows a mixed 1/2 line layout, which again, looks good to me). I think this should be consistent with the list layout outside of search. If you have a suggestion for that without dividers, I'm fine with that.

[maniac103]

If you have a suggestion for that without dividers, I'm fine with that.

That's good, I'll check how removing dividers entirely will look like. Speaking of design changes: are you opposed to reducing primary font size a bit (18sp -> 16sp)? I've done it in the context menu already to be consistent with other (popup) menus, but I think Jive item lists would also benefit from making that change to look a bit less 'busy'.

To answer the question above, this is achieved by adding the 2 lines to the ConstraintLayout XML. Link to documentation. I believe Spreadwill give the desired result. An app:layout_constraintBottom_toBottomOf="parent" also have to be added to cover art and context menu button to vertically center those, however.

Thanks for that pointer. I'll give it a try. In general I'm also in favor of reducing view count where possible, I'm just not that familiar with ConstraintLayout.

[kaaholst]

First, thanks for the contribution, it's much appreciated! I will be off until after new year, I will look into this when I get back. Just so you know if I'm not answering! I like the direction of these changes, so I think it's fine to merge. There is a new version on the way out now (it's in beta). These changes won't make it into that, they will be in the next one. To be clear about the ConstraintLayout in item_list.xml you should be able to achieve the same effect by adding the above 4 lines to the existing version. On a side note, I think vertical chains can also be used for now_playing_include.xmlusing Weighted and adding layout_constraintVertical_weight and perhaps some alignment. I'm not sure, it simplifies things though, and I have no preferences either way.

[maniac103]

To be clear about the ConstraintLayout in item_list.xml you should be able to achieve the same effect by adding the above 4 lines to the existing version. On a side note, I think vertical chains can also be used for now_playing_include.xmlusing Weighted and adding layout_constraintVertical_weight and perhaps some alignment. I'm not sure, it simplifies things though, and I have no preferences either way.

I've reverted the LinearLayout changes with one exception: now_playing_include. I tried pretty hard to make it work with ConstraintLayout, but I fail at chaining the multiple parts of the layout (title, group with item info, group with playtime slider + buttons). For grouping item info and playtime info I used Barrier, but I can't find a way to chain multiple Barriers.

[maniac103]

I like the direction of these changes, so I think it's fine to merge. There is a new version on the way out now (it's in beta). > These changes won't make it into that, they will be in the next one.

:+1: I have some more changes (some font sizing, remove divider lines, improve Jive item list header) locally. Do you prefer me adding them into this PR, or to create a follow-up PR when this one is merged?

[kaaholst]

👍 I have some more changes (some font sizing, remove divider lines, improve Jive item list header) locally. Do you prefer me adding them into this PR, or to create a follow-up PR when this one is merged?

I guess we need the divider changes at any rate, to make it consistent with the item list in the search view, so they can go in this PR. But it's up to you, either way will be ok by me. About the font size changes, would that be using @android:style/TextAppearance instead of @android:style/TextAppearance.Medium. If so, then I'm fine with it. I'm actually surprised that medium is not the standard size.

[maniac103]

No list item padding gives issues with fast scroller. Try to show a long list, e.g., My Music / Albums or All Artists. When try to tap the context menu, you will likely hit the fast scroller. See the images below. Any ideas/thoughts?

I've restored the padding in the list item style, and instead removed the side padding from the fast scroller container. That way overlap between scroller handle and overflow button should be as before, while still maintaining the desired effect of less horizontal padding. As a bonus, the ripple animation now extends to the edge of the list.

What about landscape and volume adjust and for small devices?

Landscape already had the symmetric padding, volume adjust is not a problem because it has the same background as the whole fragment (and additionally has some padding on its own).

The style SqueezerWidget.NowPlaying.CoverArt is defined in values/styles.xml for small phones and in sw340dp-long/styles.xml, which is used for phones which have the smallest width of 340dp or more and are long. See: https://developer.android.com/guide/topics/resources/providing-resources The purpose of the sw340dp-long style is an attempt to avoid getting the song-info, song-position and av-controls section too cramped on small devices.

I don't think the addtional 8dp padding should make too much of a difference, but I can double check. What's the smallest screen size you intend to support? FWIW, it seems fine on an Nexus 5X emulator.

volume_up is not a button, it's just an icon. volume_down was originally not a button either, it was converted to button when mute support was introduced.

That's exactly what made me change it: I was confused there's two things that look identical, but one is a button ('does something') and the other one isn't.

This change is inconsistent with synchronized players. The group volume slider has icons, while the volume slider for the individual players does not. If you prefer to remove the volume_up icon for the group, I'm fine with that.

I've pushed some restyling for the synchronized player list that removes the second icon there as well.

But it's up to you, either way will be ok by me.

I've added them here, let me know if there are parts you'd like to have pushed out. The last one ('Hide previous display message as soon as a new one arrives') is unrelated to this topic; I've just included it here instead of a separate MR because it conflicts with 'Beautify undo bar and display message toast.'.

[maniac103]

Player group still has volume up icon

That's correct, need to fix that.

[maniac103]

That's correct, need to fix that.

Done, removed both icons, since neither has an actual function and there's a label describing the slider right above it.

[maniac103]

Did you forget to push something?

Noticed I forgot to git add some alarm clock related drawables, fixed now.

[kaaholst]

Any news on this one? Or are you waiting for something from me?

[kaaholst]

Merged, thanks for the contribution, it is much appreciated!

[kaaholst]

I saw you removed the changes for Spinner vs AutoCompleteTextView. That also removed the changes for the alarm list, which I liked. So I also merged the alarm beautify commit, and changed so that alarm sound is selected sub-activity instead of Spinner.