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

Allow keyboard navigation #1993

Merged
merged 23 commits into from
Aug 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
67e403c
Improve tab navigation while navigating in thread list
NicolasBourdin88 Jul 17, 2024
288408e
Add right arrow navigation to go to new message button
NicolasBourdin88 Jul 17, 2024
582be0c
Make Toolbar focusable when using keyboard navigation
NicolasBourdin88 Jul 19, 2024
d00ac5f
Set avatar focusable
NicolasBourdin88 Jul 19, 2024
b4edb8c
Allow to select folder on search fragment
NicolasBourdin88 Jul 30, 2024
24e886b
Navigate in toolbar in Thread Fragment
NicolasBourdin88 Jul 30, 2024
78f4600
Unable to select avatar on ThreadListFragment
NicolasBourdin88 Jul 30, 2024
3850e85
Override setFocusable so it affect the avatar root view
NicolasBourdin88 Aug 5, 2024
1504c62
Remove focus set on all page when switching fragment
NicolasBourdin88 Aug 5, 2024
9adb04d
Reformat
NicolasBourdin88 Aug 6, 2024
07bfe07
Apply suggestion from code review
NicolasBourdin88 Aug 6, 2024
9052397
Remove useless isFocusable
NicolasBourdin88 Aug 8, 2024
8c10ecd
Avoid drawer from getting focus
NicolasBourdin88 Aug 8, 2024
cbdbe75
Remember focus after navigating from Fragment to Fragment
NicolasBourdin88 Aug 8, 2024
bc87e57
Remember focus in MailboxSettingsFragment
NicolasBourdin88 Aug 12, 2024
2b10142
Apply suggestion from code review
NicolasBourdin88 Aug 12, 2024
ff30e9a
Avoid signature Webview to get focus and get clicked
NicolasBourdin88 Aug 14, 2024
ed8533c
Reformat comment to avoid it to be > 131 characters
NicolasBourdin88 Aug 14, 2024
43ffd95
Avoid scrolling when clicking on web view
NicolasBourdin88 Aug 15, 2024
0a9ad2e
Add commentary
NicolasBourdin88 Aug 15, 2024
50ce437
Refractor
NicolasBourdin88 Aug 15, 2024
d0d732b
Clean `saveFocusWhenNavigatingBack()` code
KevinBoulongne Aug 15, 2024
d072f80
Rewrite comments
KevinBoulongne Aug 15, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion app/src/main/java/com/infomaniak/mail/ui/MainActivity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ class MainActivity : BaseActivity() {

setupSnackbar()
setupNavController()
setupMenuDrawerCallbacks()
setupMenuDrawer()

mainViewModel.updateUserInfo()

Expand All @@ -212,6 +212,11 @@ class MainActivity : BaseActivity() {
initAppReviewManager()
}

private fun setupMenuDrawer() {
binding.drawerLayout.isFocusable = false // Set here because not working in XML
setupMenuDrawerCallbacks()
}

private fun initAppReviewManager() {
inAppReviewManager.init(
onDialogShown = { trackInAppReviewEvent("presentAlert") },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,13 @@ class ThreadListAdapter @Inject constructor(

private fun CardviewThreadItemBinding.displayAvatar(thread: Thread) {
val (recipient, bimi) = thread.computeAvatarRecipient()
expeditorAvatar.loadAvatar(recipient, bimi)
expeditorAvatar.apply {
loadAvatar(recipient, bimi)

// Set `isFocusable` here instead of in XML file because setting it in the
// XML doesn't trigger the overridden `setFocusable(boolean)` in AvatarView.
isFocusable = false
}
}

private fun CardviewThreadItemBinding.formatRecipientNames(recipients: List<Recipient>): String {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ import com.infomaniak.mail.views.itemViews.SelectableFolderItemView

class SearchFolderAdapter(
val folders: List<Any>,
val onClickListener: (folder: Folder?, title: String) -> Unit,
) : ListAdapter {

private var selectedFolder: Folder? = null
Expand Down Expand Up @@ -74,13 +73,9 @@ class SearchFolderAdapter(
): View {
return (convertView ?: ItemSearchFolderBinding.inflate(LayoutInflater.from(context), parent, false).root).apply {
findViewById<SelectableFolderItemView>(R.id.simpleFolderItemView).apply {
val entryName: String = context.getLocalizedNameOrAllFolders(folder)
text = entryName
text = context.getLocalizedNameOrAllFolders(folder)
icon = AppCompatResources.getDrawable(context, folder?.getIcon() ?: R.drawable.ic_all_folders)

setSelectedState(folder == selectedFolder)

setOnClickListener { onClickListener(folder, entryName) }
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,13 +206,21 @@ class SearchFragment : TwoPaneFragment() {
.apply { add(0, SearchFolderElement.ALL_FOLDERS) }
.toList()

searchAdapter = SearchFolderAdapter(folders) { folder, title ->
onFolderSelected(folder, title)
popupMenu.dismiss()
}
searchAdapter = SearchFolderAdapter(folders)

popupMenu.setAdapter(searchAdapter)

popupMenu.setOnItemClickListener { _, _, position, _ ->
if (searchAdapter.getItemViewType(position) != SearchFolderElement.DIVIDER.itemId) {

val folder = folders[position] as? Folder
val entryName = requireContext().getLocalizedNameOrAllFolders(folder)

onFolderSelected(folder, entryName)
popupMenu.dismiss()
}
}

updateFolderDropDownUi(
folder = searchViewModel.filterFolder,
title = requireContext().getLocalizedNameOrAllFolders(searchViewModel.filterFolder),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import com.infomaniak.mail.data.LocalSettings
import com.infomaniak.mail.data.models.FeatureFlag
import com.infomaniak.mail.databinding.FragmentSettingsBinding
import com.infomaniak.mail.ui.MainViewModel
import com.infomaniak.mail.utils.UiUtils.saveFocusWhenNavigatingBack
import com.infomaniak.mail.utils.extensions.animatedNavigation
import com.infomaniak.mail.utils.extensions.launchSyncAutoConfigActivityForResult
import com.infomaniak.mail.utils.extensions.observeNotNull
Expand All @@ -53,6 +54,11 @@ class SettingsFragment : Fragment() {
@Inject
lateinit var localSettings: LocalSettings

override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
saveFocusWhenNavigatingBack(getLayout = { binding.linearLayoutContainer }, lifecycle)
}

override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View {
return FragmentSettingsBinding.inflate(inflater, container, false).also { binding = it }.root
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import com.infomaniak.mail.R
import com.infomaniak.mail.data.LocalSettings
import com.infomaniak.mail.data.LocalSettings.SwipeAction
import com.infomaniak.mail.databinding.FragmentSwipeActionsSettingsBinding
import com.infomaniak.mail.utils.UiUtils.saveFocusWhenNavigatingBack
import com.infomaniak.mail.utils.extensions.animatedNavigation
import com.infomaniak.mail.utils.extensions.setSystemBarsColors
import dagger.hilt.android.AndroidEntryPoint
Expand All @@ -43,6 +44,11 @@ class SwipeActionsSettingsFragment : Fragment() {
@Inject
lateinit var localSettings: LocalSettings

override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
saveFocusWhenNavigatingBack(getLayout = { binding.linearLayoutContainer }, lifecycle)
}

override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View {
return FragmentSwipeActionsSettingsBinding.inflate(inflater, container, false).also { binding = it }.root
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import com.infomaniak.lib.core.utils.safeBinding
import com.infomaniak.mail.R
import com.infomaniak.mail.databinding.FragmentMailboxSettingsBinding
import com.infomaniak.mail.ui.main.settings.ItemSettingView
import com.infomaniak.mail.utils.UiUtils.saveFocusWhenNavigatingBack
import com.infomaniak.mail.utils.extensions.animatedNavigation
import com.infomaniak.mail.utils.extensions.notYetImplemented
import com.infomaniak.mail.utils.extensions.setSystemBarsColors
Expand All @@ -36,6 +37,11 @@ class MailboxSettingsFragment : Fragment() {
private var binding: FragmentMailboxSettingsBinding by safeBinding()
private val navigationArgs: MailboxSettingsFragmentArgs by navArgs()

override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
saveFocusWhenNavigatingBack(getLayout = { binding.linearLayoutContainer }, lifecycle)
}

override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View {
return FragmentMailboxSettingsBinding.inflate(inflater, container, false).also { binding = it }.root
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import com.infomaniak.lib.core.utils.safeBinding
import com.infomaniak.mail.BuildConfig
import com.infomaniak.mail.MatomoMail.trackEvent
import com.infomaniak.mail.databinding.FragmentDataManagementSettingsBinding
import com.infomaniak.mail.utils.UiUtils.saveFocusWhenNavigatingBack
import com.infomaniak.mail.utils.extensions.animatedNavigation
import com.infomaniak.mail.utils.extensions.setSystemBarsColors
import dagger.hilt.android.AndroidEntryPoint
Expand All @@ -37,6 +38,11 @@ class DataManagementSettingsFragment : Fragment() {

private var binding: FragmentDataManagementSettingsBinding by safeBinding()

override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
saveFocusWhenNavigatingBack(getLayout = { binding.linearLayoutContainer }, lifecycle)
}

override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View {
return FragmentDataManagementSettingsBinding.inflate(inflater, container, false).also { binding = it }.root
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import com.infomaniak.mail.utils.ConfettiUtils
import com.infomaniak.mail.utils.ConfettiUtils.ConfettiType
import com.infomaniak.mail.utils.LogoutUser
import com.infomaniak.mail.utils.PlayServicesUtils
import com.infomaniak.mail.utils.UiUtils.saveFocusWhenNavigatingBack
import com.infomaniak.mail.utils.extensions.animatedNavigation
import com.infomaniak.mail.utils.extensions.bindAlertToViewLifecycle
import com.infomaniak.mail.utils.extensions.setSystemBarsColors
Expand Down Expand Up @@ -83,6 +84,11 @@ class AccountFragment : Fragment(), MailboxListFragment {
@Inject
lateinit var descriptionDialog: DescriptionAlertDialog

override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
saveFocusWhenNavigatingBack(getLayout = { binding.constraintLayoutContainer }, lifecycle)
}

override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View {
return FragmentAccountBinding.inflate(inflater, container, false).also { binding = it }.root
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,10 @@ class NewMessageFragment : Fragment() {
fromMailAddress.apply {
icon = AppCompatResources.getDrawable(context, R.drawable.ic_chevron_down)
setOnClickListener { _ -> addressListPopupWindow?.show() }

// Set `isFocusable` here instead of in XML file because setting it in the
// XML doesn't trigger the overridden `setFocusable(boolean)` in AvatarView.
isFocusable = true
}
}
}
Expand Down
32 changes: 32 additions & 0 deletions app/src/main/java/com/infomaniak/mail/utils/UiUtils.kt
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,20 @@ import android.animation.ValueAnimator
import android.content.Context
import android.graphics.Color
import android.os.Build
import android.view.View
import android.view.ViewGroup
import android.view.Window
import android.widget.TextView
import androidx.annotation.ColorInt
import androidx.annotation.FloatRange
import androidx.annotation.IdRes
import androidx.appcompat.content.res.AppCompatResources
import androidx.core.graphics.toColor
import androidx.core.graphics.toColorInt
import androidx.core.view.isGone
import androidx.lifecycle.DefaultLifecycleObserver
import androidx.lifecycle.Lifecycle
import androidx.lifecycle.LifecycleOwner
import com.infomaniak.mail.R
import com.infomaniak.mail.data.models.correspondent.Correspondent
import com.infomaniak.mail.utils.extensions.updateNavigationBarColor
Expand Down Expand Up @@ -157,4 +163,30 @@ object UiUtils {
}

fun dividerDrawable(context: Context) = AppCompatResources.getDrawable(context, R.drawable.divider)

fun saveFocusWhenNavigatingBack(getLayout: () -> ViewGroup, lifecycle: Lifecycle) {

val lifecycleObserver = object : DefaultLifecycleObserver {

@IdRes
private var lastFocusViewId: Int? = null

override fun onStart(owner: LifecycleOwner) {
super.onStart(owner)
lastFocusViewId?.let { viewId -> getLayout().findViewById<View>(viewId).requestFocus() }
}

override fun onStop(owner: LifecycleOwner) {
getLayout().focusedChild?.let { lastFocusViewId = it.id }
super.onStop(owner)
}

override fun onDestroy(owner: LifecycleOwner) {
lifecycle.removeObserver(observer = this)
super.onDestroy(owner)
}
}

lifecycle.addObserver(lifecycleObserver)
}
}
4 changes: 4 additions & 0 deletions app/src/main/java/com/infomaniak/mail/views/AvatarView.kt
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,10 @@ class AvatarView @JvmOverloads constructor(
binding.root.setOnLongClickListener(onLongClickListener)
}

override fun setFocusable(focusable: Boolean) {
binding.root.isFocusable = focusable
}

fun loadAvatar(user: User) = with(binding.avatarImage) {
contentDescription = user.email
loadAvatar(
Expand Down
1 change: 1 addition & 0 deletions app/src/main/res/layout/cardview_thread_item.xml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
android:layout_height="wrap_content"
android:layout_marginVertical="2dp"
android:layout_marginStart="@dimen/marginStandardVerySmall"
android:nextFocusRight="@+id/newMessageFab"
app:cardCornerRadius="@null"
app:cardPreventCornerOverlap="false"
app:shapeAppearanceOverlay="@style/RoundedDecoratedTextItemShapeAppearance">
Expand Down
1 change: 1 addition & 0 deletions app/src/main/res/layout/fragment_account.xml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
android:layout_weight="1">

<androidx.constraintlayout.widget.ConstraintLayout
android:id="@+id/constraintLayoutContainer"
android:layout_width="match_parent"
android:layout_height="match_parent">

Expand Down
1 change: 1 addition & 0 deletions app/src/main/res/layout/fragment_ai_engine_choice.xml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
android:id="@+id/toolbar"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:touchscreenBlocksFocus="false"
app:navigationIconTint="@color/iconColor" />

<TextView
Expand Down
1 change: 1 addition & 0 deletions app/src/main/res/layout/fragment_ai_prompt.xml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
android:layout_width="0dp"
android:layout_height="wrap_content"
android:clickable="true"
android:focusable="false"
android:foregroundTint="@android:color/transparent"
app:layout_constraintBottom_toBottomOf="parent"
app:layout_constraintEnd_toEndOf="parent"
Expand Down
2 changes: 2 additions & 0 deletions app/src/main/res/layout/fragment_ai_proposition.xml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
android:id="@+id/toolbar"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:touchscreenBlocksFocus="false"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent"
Expand Down Expand Up @@ -66,6 +67,7 @@
android:id="@+id/contentLayout"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:focusable="true"
android:paddingHorizontal="@dimen/marginStandardMedium"
android:paddingVertical="@dimen/marginStandard">

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
android:layout_height="match_parent">

<LinearLayout
android:id="@+id/linearLayoutContainer"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:orientation="vertical">
Expand Down
1 change: 1 addition & 0 deletions app/src/main/res/layout/fragment_mailbox_settings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
android:scrollbars="none">

<LinearLayout
android:id="@+id/linearLayoutContainer"
android:layout_width="match_parent"
android:layout_height="match_parent"
android:clipToPadding="false"
Expand Down
1 change: 1 addition & 0 deletions app/src/main/res/layout/fragment_move.xml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
android:id="@+id/toolbar"
android:layout_width="match_parent"
android:layout_height="?attr/actionBarSize"
android:touchscreenBlocksFocus="false"
app:contentInsetEnd="0dp">

<LinearLayout
Expand Down
4 changes: 4 additions & 0 deletions app/src/main/res/layout/fragment_new_message.xml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
android:layout_width="match_parent"
android:layout_height="?attr/actionBarSize"
android:background="@color/newMessageBackgroundColor"
android:touchscreenBlocksFocus="false"
app:contentInsetEnd="@dimen/marginStandardSmall"
app:layout_constraintBottom_toTopOf="@+id/compositionNestedScrollView"
app:layout_constraintTop_toTopOf="parent"
Expand Down Expand Up @@ -368,6 +369,8 @@
android:layout_height="wrap_content"
android:layout_marginTop="-16dp"
android:layout_marginBottom="@dimen/marginStandardMedium"
android:focusable="false"
android:focusableInTouchMode="false"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toBottomOf="@id/editorWebView"
Expand Down Expand Up @@ -405,6 +408,7 @@
android:id="@+id/quoteWebView"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:focusableInTouchMode="false"
android:visibility="gone"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toStartOf="parent"
Expand Down
7 changes: 5 additions & 2 deletions app/src/main/res/layout/fragment_search.xml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@

<com.google.android.material.appbar.AppBarLayout
android:layout_width="match_parent"
android:layout_height="wrap_content">
android:layout_height="wrap_content"
android:touchscreenBlocksFocus="false">

<com.google.android.material.appbar.CollapsingToolbarLayout
android:id="@+id/collapsingToolbarLayout"
Expand All @@ -44,6 +45,7 @@
android:id="@+id/toolbar"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:touchscreenBlocksFocus="false"
app:contentInsetEnd="@dimen/marginStandardMedium"
app:layout_collapseMode="pin">

Expand Down Expand Up @@ -111,7 +113,6 @@
android:id="@+id/folderDropDown"
android:layout_width="wrap_content"
android:layout_height="match_parent"
android:backgroundTint="@color/chip_unread_background"
android:checkable="true"
android:drawableTint="@color/chip_unread_text_color"
android:ellipsize="end"
Expand All @@ -125,7 +126,9 @@
android:paddingEnd="@dimen/marginStandardMedium"
android:text="@string/searchFilterFolder"
android:textColor="@color/chip_unread_text_color"
app:backgroundTint="@color/chip_unread_background"
app:closeIcon="@drawable/ic_chevron_down"
app:rippleColor="?attr/colorControlHighlight"
app:strokeColor="?attr/colorPrimary"
app:strokeWidth="1dp"
app:toggleCheckedStateOnClick="false"
Expand Down
1 change: 1 addition & 0 deletions app/src/main/res/layout/fragment_settings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
android:scrollbars="none">

<LinearLayout
android:id="@+id/linearLayoutContainer"
android:layout_width="match_parent"
android:layout_height="match_parent"
android:clipToPadding="false"
Expand Down
Loading
Loading