From 630950ea5d69d6198afbff0b2018039baa071148 Mon Sep 17 00:00:00 2001 From: OxygenCobalt Date: Wed, 22 Jun 2022 12:31:52 -0600 Subject: [PATCH] playback: force LTR on timeline controls Force LTR on timeline controls, as per the Material Design guidelines. The guidelines state that while "directional" UIs should be LTR/RTL depending on locale, "timeline" UIs should always by LTR, as the direction of time is universal. Auxio did not do this, and so the timeline controls would be RTL on other elements. Fix this by forcing LTR on the UI elements that correspond to timelines. Now, this is not the best system. To ensure that the rest of the layout remains sane, much of the directional views have to be wrapped in a redundant layout, which is somewhat in-efficient. However, the impact seems to be at least negligable. --- CHANGELOG.md | 3 + .../java/org/oxycblt/auxio/MainActivity.kt | 2 - .../auxio/detail/AlbumDetailFragment.kt | 6 +- .../auxio/detail/ArtistDetailFragment.kt | 6 +- .../auxio/detail/GenreDetailFragment.kt | 6 +- .../auxio/home/list/SongListFragment.kt | 2 +- .../auxio/playback/NoRtlFrameLayout.kt | 46 ++++++ .../auxio/playback/PlaybackBarFragment.kt | 7 +- .../auxio/playback/PlaybackPanelFragment.kt | 2 - .../oxycblt/auxio/playback/StyledSeekBar.kt | 7 +- .../java/org/oxycblt/auxio/widgets/Forms.kt | 4 + .../layout-land/fragment_playback_panel.xml | 138 +++++++++-------- .../fragment_playback_panel.xml | 135 +++++++++-------- .../layout-sw600dp/fragment_playback_bar.xml | 73 +++++---- .../fragment_playback_panel.xml | 138 +++++++++-------- .../layout-sw600dp/view_playback_controls.xml | 80 ++++++++++ .../fragment_playback_panel.xml | 141 ++++++++++-------- .../main/res/layout/fragment_playback_bar.xml | 71 +++++---- .../res/layout/fragment_playback_panel.xml | 138 +++++++++-------- .../res/layout/view_playback_controls.xml | 79 ++++++++++ app/src/main/res/layout/view_seek_bar.xml | 7 +- app/src/main/res/layout/widget_large.xml | 1 + app/src/main/res/layout/widget_medium.xml | 1 + app/src/main/res/layout/widget_small.xml | 3 +- app/src/main/res/layout/widget_thin.xml | 1 - app/src/main/res/layout/widget_wide.xml | 3 +- 26 files changed, 710 insertions(+), 390 deletions(-) create mode 100644 app/src/main/java/org/oxycblt/auxio/playback/NoRtlFrameLayout.kt create mode 100644 app/src/main/res/layout-sw600dp/view_playback_controls.xml create mode 100644 app/src/main/res/layout/view_playback_controls.xml diff --git a/CHANGELOG.md b/CHANGELOG.md index 22a4359cd..6a92cb6e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ - Added a shuffle shortcut - You can now customize what occurs when a song is played from an album/artist/genre [#164] +#### What's Improved +- Made "timeline" elements (like playback controls) always left-to-right + #### What's Fixed - Fixed broken tablet layouts - Fixed seam that would appear on some album covers diff --git a/app/src/main/java/org/oxycblt/auxio/MainActivity.kt b/app/src/main/java/org/oxycblt/auxio/MainActivity.kt index cf8e10228..9f9321049 100644 --- a/app/src/main/java/org/oxycblt/auxio/MainActivity.kt +++ b/app/src/main/java/org/oxycblt/auxio/MainActivity.kt @@ -43,8 +43,6 @@ import org.oxycblt.auxio.util.logD * * TODO: Custom language support * - * TODO: Rework padding ethos - * * TODO: Add multi-select * * @author OxygenCobalt diff --git a/app/src/main/java/org/oxycblt/auxio/detail/AlbumDetailFragment.kt b/app/src/main/java/org/oxycblt/auxio/detail/AlbumDetailFragment.kt index 368e150b2..860ac391f 100644 --- a/app/src/main/java/org/oxycblt/auxio/detail/AlbumDetailFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/detail/AlbumDetailFragment.kt @@ -144,15 +144,15 @@ class AlbumDetailFragment : override fun onShowSortMenu(anchor: View) { menu(anchor, R.menu.menu_album_sort) { val sort = detailModel.albumSort - requireNotNull(menu.findItem(sort.mode.itemId)).isChecked = true - requireNotNull(menu.findItem(R.id.option_sort_asc)).isChecked = sort.isAscending + unlikelyToBeNull(menu.findItem(sort.mode.itemId)).isChecked = true + unlikelyToBeNull(menu.findItem(R.id.option_sort_asc)).isChecked = sort.isAscending setOnMenuItemClickListener { item -> item.isChecked = !item.isChecked detailModel.albumSort = if (item.itemId == R.id.option_sort_asc) { sort.withAscending(item.isChecked) } else { - sort.withMode(requireNotNull(Sort.Mode.fromItemId(item.itemId))) + sort.withMode(unlikelyToBeNull(Sort.Mode.fromItemId(item.itemId))) } true } diff --git a/app/src/main/java/org/oxycblt/auxio/detail/ArtistDetailFragment.kt b/app/src/main/java/org/oxycblt/auxio/detail/ArtistDetailFragment.kt index 1a3a78b1f..dea0bf89c 100644 --- a/app/src/main/java/org/oxycblt/auxio/detail/ArtistDetailFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/detail/ArtistDetailFragment.kt @@ -139,8 +139,8 @@ class ArtistDetailFragment : override fun onShowSortMenu(anchor: View) { menu(anchor, R.menu.menu_artist_sort) { val sort = detailModel.artistSort - requireNotNull(menu.findItem(sort.mode.itemId)).isChecked = true - requireNotNull(menu.findItem(R.id.option_sort_asc)).isChecked = sort.isAscending + unlikelyToBeNull(menu.findItem(sort.mode.itemId)).isChecked = true + unlikelyToBeNull(menu.findItem(R.id.option_sort_asc)).isChecked = sort.isAscending setOnMenuItemClickListener { item -> item.isChecked = !item.isChecked @@ -148,7 +148,7 @@ class ArtistDetailFragment : if (item.itemId == R.id.option_sort_asc) { sort.withAscending(item.isChecked) } else { - sort.withMode(requireNotNull(Sort.Mode.fromItemId(item.itemId))) + sort.withMode(unlikelyToBeNull(Sort.Mode.fromItemId(item.itemId))) } true diff --git a/app/src/main/java/org/oxycblt/auxio/detail/GenreDetailFragment.kt b/app/src/main/java/org/oxycblt/auxio/detail/GenreDetailFragment.kt index e419881d8..8b2aac401 100644 --- a/app/src/main/java/org/oxycblt/auxio/detail/GenreDetailFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/detail/GenreDetailFragment.kt @@ -140,15 +140,15 @@ class GenreDetailFragment : override fun onShowSortMenu(anchor: View) { menu(anchor, R.menu.menu_genre_sort) { val sort = detailModel.genreSort - requireNotNull(menu.findItem(sort.mode.itemId)).isChecked = true - requireNotNull(menu.findItem(R.id.option_sort_asc)).isChecked = sort.isAscending + unlikelyToBeNull(menu.findItem(sort.mode.itemId)).isChecked = true + unlikelyToBeNull(menu.findItem(R.id.option_sort_asc)).isChecked = sort.isAscending setOnMenuItemClickListener { item -> item.isChecked = !item.isChecked detailModel.genreSort = if (item.itemId == R.id.option_sort_asc) { sort.withAscending(item.isChecked) } else { - sort.withMode(requireNotNull(Sort.Mode.fromItemId(item.itemId))) + sort.withMode(unlikelyToBeNull(Sort.Mode.fromItemId(item.itemId))) } true } diff --git a/app/src/main/java/org/oxycblt/auxio/home/list/SongListFragment.kt b/app/src/main/java/org/oxycblt/auxio/home/list/SongListFragment.kt index f03cd8567..2b6b29633 100644 --- a/app/src/main/java/org/oxycblt/auxio/home/list/SongListFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/home/list/SongListFragment.kt @@ -27,8 +27,8 @@ import org.oxycblt.auxio.ui.DisplayMode import org.oxycblt.auxio.ui.Item import org.oxycblt.auxio.ui.MenuItemListener import org.oxycblt.auxio.ui.MonoAdapter -import org.oxycblt.auxio.ui.Sort import org.oxycblt.auxio.ui.SongViewHolder +import org.oxycblt.auxio.ui.Sort import org.oxycblt.auxio.ui.SyncBackingData import org.oxycblt.auxio.util.collectImmediately import org.oxycblt.auxio.util.context diff --git a/app/src/main/java/org/oxycblt/auxio/playback/NoRtlFrameLayout.kt b/app/src/main/java/org/oxycblt/auxio/playback/NoRtlFrameLayout.kt new file mode 100644 index 000000000..a561f592a --- /dev/null +++ b/app/src/main/java/org/oxycblt/auxio/playback/NoRtlFrameLayout.kt @@ -0,0 +1,46 @@ +/* + * Copyright (c) 2022 Auxio Project + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +package org.oxycblt.auxio.playback + +import android.content.Context +import android.util.AttributeSet +import android.view.View +import android.widget.FrameLayout + +/** + * A class that programmatically overrides the child layout to a left-to-right (LTR) layout + * direction. + * + * The Material Design guidelines state that any components that represent a "Timeline" should + * always be LTR. In Auxio, this applies to most of the playback components. This layout in + * particular overrides the layout direction in a way that will not disrupt how other views are laid + * out. + */ +open class NoRtlFrameLayout +@JvmOverloads +constructor( + context: Context, + attrs: AttributeSet? = null, + defStyleAttr: Int = 0, +) : FrameLayout(context, attrs, defStyleAttr) { + override fun onFinishInflate() { + super.onFinishInflate() + check(childCount == 1) { "This view should only contain one child" } + getChildAt(0).layoutDirection = View.LAYOUT_DIRECTION_LTR + } +} diff --git a/app/src/main/java/org/oxycblt/auxio/playback/PlaybackBarFragment.kt b/app/src/main/java/org/oxycblt/auxio/playback/PlaybackBarFragment.kt index 3913447c8..5b9eb03da 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/PlaybackBarFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/PlaybackBarFragment.kt @@ -73,10 +73,9 @@ class PlaybackBarFragment : ViewBindingFragment() { // Load the track color in manually as it's unclear whether the track actually supports // using a ColorStateList in the resources - binding.playbackProgressBar.trackColor = - requireContext().getColorStateListSafe(R.color.sel_track).defaultColor - - binding.playbackProgressBar.progress = 0 + binding.playbackProgressBar.apply { + trackColor = requireContext().getColorStateListSafe(R.color.sel_track).defaultColor + } binding.playbackPlayPause.setOnClickListener { playbackModel.invertPlaying() } diff --git a/app/src/main/java/org/oxycblt/auxio/playback/PlaybackPanelFragment.kt b/app/src/main/java/org/oxycblt/auxio/playback/PlaybackPanelFragment.kt index f4a8e098f..71083a657 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/PlaybackPanelFragment.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/PlaybackPanelFragment.kt @@ -46,8 +46,6 @@ import org.oxycblt.auxio.util.textSafe * Instantiation is done by the navigation component, **do not instantiate this fragment manually.** * @author OxygenCobalt * - * TODO: Handle RTL correctly in the playback buttons - * * TODO: Make seek thumb grow when selected */ class PlaybackPanelFragment : diff --git a/app/src/main/java/org/oxycblt/auxio/playback/StyledSeekBar.kt b/app/src/main/java/org/oxycblt/auxio/playback/StyledSeekBar.kt index e5e08e6a8..f287a77fd 100644 --- a/app/src/main/java/org/oxycblt/auxio/playback/StyledSeekBar.kt +++ b/app/src/main/java/org/oxycblt/auxio/playback/StyledSeekBar.kt @@ -19,7 +19,6 @@ package org.oxycblt.auxio.playback import android.content.Context import android.util.AttributeSet -import android.widget.FrameLayout import com.google.android.material.slider.Slider import kotlin.math.max import org.oxycblt.auxio.databinding.ViewSeekBarBinding @@ -52,12 +51,14 @@ constructor( attrs: AttributeSet? = null, defStyleAttr: Int = 0, ) : - FrameLayout(context, attrs, defStyleAttr), + NoRtlFrameLayout(context, attrs, defStyleAttr), Slider.OnSliderTouchListener, Slider.OnChangeListener { - private val binding = ViewSeekBarBinding.inflate(context.inflater, this) + private val binding = ViewSeekBarBinding.inflate(context.inflater, this, true) init { + // As per the Material Design guidelines, timeline elements like SeekBars and Controls + // should always be LTR. binding.seekBarSlider.addOnSliderTouchListener(this) binding.seekBarSlider.addOnChangeListener(this) } diff --git a/app/src/main/java/org/oxycblt/auxio/widgets/Forms.kt b/app/src/main/java/org/oxycblt/auxio/widgets/Forms.kt index b584635b1..902f774ec 100644 --- a/app/src/main/java/org/oxycblt/auxio/widgets/Forms.kt +++ b/app/src/main/java/org/oxycblt/auxio/widgets/Forms.kt @@ -18,6 +18,7 @@ package org.oxycblt.auxio.widgets import android.content.Context +import android.view.View import android.widget.RemoteViews import androidx.annotation.LayoutRes import org.oxycblt.auxio.R @@ -114,6 +115,9 @@ private fun RemoteViews.applyPlayPauseControls( R.id.widget_play_pause, context.newBroadcastPendingIntent(PlaybackService.ACTION_PLAY_PAUSE)) + // Controls are timeline elements, override the layout direction to RTL + setInt(R.id.widget_controls, "setLayoutDirection", View.LAYOUT_DIRECTION_LTR) + setImageViewResource( R.id.widget_play_pause, if (state.isPlaying) { diff --git a/app/src/main/res/layout-land/fragment_playback_panel.xml b/app/src/main/res/layout-land/fragment_playback_panel.xml index acb50b80b..5d20bf604 100644 --- a/app/src/main/res/layout-land/fragment_playback_panel.xml +++ b/app/src/main/res/layout-land/fragment_playback_panel.xml @@ -81,73 +81,91 @@ android:id="@+id/playback_seek_bar" android:layout_width="match_parent" android:layout_height="wrap_content" - app:layout_constraintBottom_toTopOf="@+id/playback_play_pause" + app:layout_constraintBottom_toTopOf="@+id/playback_controls_container" app:layout_constraintEnd_toEndOf="@+id/playback_song_container" app:layout_constraintHorizontal_bias="0.0" app:layout_constraintStart_toStartOf="parent" /> - - -