Skip to content

fix: improve keyboard navigation in search conversations [WPB-14665] [WPB-14778]#4837

Open
Garzas wants to merge 5 commits into
developfrom
fix/keyboard-navigation-conversations
Open

fix: improve keyboard navigation in search conversations [WPB-14665] [WPB-14778]#4837
Garzas wants to merge 5 commits into
developfrom
fix/keyboard-navigation-conversations

Conversation

@Garzas
Copy link
Copy Markdown
Contributor

@Garzas Garzas commented May 19, 2026

https://wearezeta.atlassian.net/browse/WPB-14665


PR Submission Checklist for internal contributors

  • The PR Title

    • conforms to the style of semantic commits messages¹ supported in Wire's Github Workflow²
    • contains a reference JIRA issue number like SQPIT-764
    • answers the question: If merged, this PR will: ... ³
  • The PR Description

    • is free of optional paragraphs and you have filled the relevant parts to the best of your ability

What's new in this PR?

Issues

Fixed issues:

  • Search fields could receive keyboard input, but search results and related controls were not reliably reachable or actionable with keyboard navigation.
  • The home search field could trap focus or make surrounding controls, such as profile, clear search, and New/FAB, difficult to reach in the expected TAB order.
  • The New Conversation button shown for empty search results could not be reached reliably with keyboard navigation.
  • The clear x button in the search field was skipped by keyboard focus in some search states.
  • Some controls were reachable only backwards with SHIFT+TAB, but not forwards with TAB.

Causes

The affected Compose search components relied mostly on default focus traversal. This was not enough for dynamic UI states such as inactive/active search, conditionally visible clear actions, empty search results, and FAB visibility.

Some focus behavior also needed to be attached directly to the actual text input or focusable control so keyboard traversal would not skip intermediate actions.

Solutions

  • Added explicit focus requesters and focusProperties for the Conversations home top bar, profile action, FAB/New button, search field, clear search button, and search back button.
  • Updated search handling so keyboard focus opens the search field, shows the keyboard, supports backwards focus to the back button, and allows TAB to continue into the conversation/search results list.
  • Updated empty search results so the New Conversation action is reachable from the search field with keyboard navigation.
  • Updated search clear action focus handling so TAB can move from the search field to the x button when text is present.
  • Added reusable onEscapeOrBackKey modifier used by search UI to close keyboard-driven search state consistently.

@Garzas Garzas requested review from saleniuk and sbakhtiarov May 19, 2026 13:53
@Garzas Garzas self-assigned this May 19, 2026
canSendMessage: Boolean,
onSendButtonClicked: () -> Unit,
) {
if (event.isShiftPressed) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably disable "send on enter" when input field is in "expanded" mode.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed changes from MessageComposer, because it will need to be a separate refactor

@Garzas Garzas force-pushed the fix/keyboard-navigation-conversations branch from 522503c to 384f202 Compare May 21, 2026 09:40
@Garzas Garzas changed the title fix: keyboard navigation in conversations UI [WPB-14665] fix: improve keyboard navigation in search conversations [WPB-14665] May 21, 2026
@Garzas Garzas changed the title fix: improve keyboard navigation in search conversations [WPB-14665] fix: improve keyboard navigation in search conversations [WPB-14665] [WPB-14778] May 21, 2026
@Garzas Garzas requested a review from sbakhtiarov May 21, 2026 10:10
@github-actions
Copy link
Copy Markdown
Contributor

@Garzas looks like you are rolling back kalium to a previous commitish.

This means that the PR's target branch (develop) is using a newer version of Kalium, and the changes in this PR will rollback Kalium to an older version.

develop This PR
122f2bd1c5e31459d5d090aeeee0a808064a5ef1 d62cb852afb2815902e1d61041d884c561e5f286

Is this intentional?

@codecov
Copy link
Copy Markdown

codecov Bot commented May 21, 2026

Codecov Report

❌ Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 51.28%. Comparing base (d7e2257) to head (10d4bb1).

Files with missing lines Patch % Lines
...main/kotlin/com/wire/android/ui/home/HomeTopBar.kt 0.00% 3 Missing ⚠️
...kotlin/com/wire/android/ui/home/HomeStateHolder.kt 0.00% 1 Missing ⚠️

❌ Your patch check has failed because the patch coverage (0.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #4837      +/-   ##
===========================================
- Coverage    51.29%   51.28%   -0.01%     
===========================================
  Files          611      612       +1     
  Lines        21201    21205       +4     
  Branches      3407     3409       +2     
===========================================
  Hits         10875    10875              
- Misses        9310     9314       +4     
  Partials      1016     1016              
Files with missing lines Coverage Δ
...kotlin/com/wire/android/ui/home/HomeStateHolder.kt 0.00% <0.00%> (ø)
...main/kotlin/com/wire/android/ui/home/HomeTopBar.kt 0.00% <0.00%> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d7e2257...10d4bb1. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sonarqubecloud
Copy link
Copy Markdown

searchBarState.isSearchActive -> SearchConversationsEmptyContent(onNewConversationClicked = onNewConversationClicked)
searchBarState.isSearchActive -> SearchConversationsEmptyContent(
onNewConversationClicked = onNewConversationClicked,
newConversationFocusRequester = emptySearchResultFocusRequester
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to explicitly pass it as a parameter to the SearchConversationsEmptyContent? Inside that composable it just adds it to the modifier so we could do that here directly:

SearchConversationsEmptyContent(
    onNewConversationClicked = onNewConversationClicked,
    modifier = emptySearchResultFocusRequester?.let { Modifier.focusRequester(it) } ?: Modifier,
)

Comment on lines +59 to +60
searchFocusRequester: FocusRequester? = null,
fabFocusRequester: FocusRequester? = null,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to have only single parameter and make HomeTopBar not having the logic of deciding itself which one should be used next? For instance if we use this top bar in another scenario where we have completely different elements on a screen then we will need to add more focusRequesters. 🤔
Instead, we could pass single nextFocusRequester and move the logic of deciding which one it should be to the parent, because now it feels like some part of the logic which decides which one to use next is here inside HomeTopBar and some is in the parent for instance when passing parameters:
fabFocusRequester = if (currentNavigationItem.fab != null) fabFocusRequester else null,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants