Skip to content

[CLX-3537][Horizon] Inbox a11y #3443

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.

Sign up for GitHub

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

Merged
domonkosadam merged 7 commits into from
Dec 15, 2025
Merged

[CLX-3537][Horizon] Inbox a11y #3443

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
942cfe9
Refactor inbox details
domonkosadam Dec 10, 2025
ae0811c
Attachments a11y improvements
domonkosadam Dec 10, 2025
a4adfc0
Compose screen a11y
domonkosadam Dec 10, 2025
6d75469
Fix dropdowns
domonkosadam Dec 11, 2025
fbca632
Fix tests
domonkosadam Dec 11, 2025
a8d48da
Fix findings
domonkosadam Dec 12, 2025
1677a00
Merge branch 'master' into CLX-3537-Inbox-a11y
domonkosadam Dec 15, 2025
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
2 changes: 1 addition & 1 deletion ...c/androidTest/java/com/instructure/horizon/ui/features/inbox/HorizonInboxComposeUiTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class HorizonInboxComposeUiTest {
HorizonInboxComposeScreen(uiState, pickerState, rememberNavController())
}

composeTestRule.onNodeWithContentDescription("Select Course")
composeTestRule.onNodeWithContentDescription("Course")
.assertIsDisplayed()
}

Expand Down
15 changes: 4 additions & 11 deletions ...src/main/java/com/instructure/horizon/features/inbox/compose/HorizonInboxComposeScreen.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,6 @@ import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.platform.LocalContext
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.semantics.contentDescription
import androidx.compose.ui.semantics.semantics
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.unit.dp
import androidx.core.content.ContextCompat
Expand Down Expand Up @@ -168,13 +166,14 @@ private fun HorizonInboxComposeTopBar(
stringResource(R.string.inboxComposeTitle),
style = HorizonTypography.h2,
color = HorizonColors.Text.title(),
modifier = Modifier.padding(horizontal = 12.dp)
modifier = Modifier
.padding(horizontal = 12.dp)
)
},
actions = {
IconButton(
iconRes = R.drawable.close,
contentDescription = null,
contentDescription = stringResource(R.string.close),
color = IconButtonColor.Inverse,
elevation = HorizonElevation.level4,
onClick = {
Expand Down Expand Up @@ -274,13 +273,7 @@ private fun CourseRecipientPickerSection(state: HorizonInboxComposeUiState) {
onMenuOpenChanged = { isRecipientPickerOpened = it },
minSearchQueryLengthForMenu = state.minQueryLength
)
val context = LocalContext.current
MultiSelectSearch(
recipientPickerState,
Modifier.semantics {
contentDescription = context.getString(R.string.a11y_inboxComposeSelectCourse)
}
)
MultiSelectSearch(recipientPickerState)

HorizonSpace(SpaceSize.SPACE_12)
}
Expand Down
39 changes: 28 additions & 11 deletions ...src/main/java/com/instructure/horizon/features/inbox/details/HorizonInboxDetailsScreen.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import androidx.annotation.DrawableRes
import androidx.compose.animation.AnimatedContent
import androidx.compose.foundation.ExperimentalFoundationApi
import androidx.compose.foundation.background
import androidx.compose.foundation.layout.Arrangement
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.PaddingValues
Expand All @@ -31,7 +32,8 @@ import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.layout.size
import androidx.compose.foundation.lazy.LazyColumn
import androidx.compose.foundation.lazy.items
import androidx.compose.foundation.lazy.itemsIndexed
import androidx.compose.foundation.lazy.rememberLazyListState
import androidx.compose.material3.CenterAlignedTopAppBar
import androidx.compose.material3.ExperimentalMaterial3Api
import androidx.compose.material3.Icon
Expand All @@ -50,6 +52,8 @@ import androidx.compose.ui.draw.clip
import androidx.compose.ui.platform.LocalContext
import androidx.compose.ui.res.painterResource
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.semantics.isTraversalGroup
import androidx.compose.ui.semantics.semantics
import androidx.compose.ui.text.input.TextFieldValue
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.unit.dp
Expand Down Expand Up @@ -198,25 +202,39 @@ private fun HorizonInboxDetailsContent(
.clip(HorizonCornerRadius.level4Top)
.background(HorizonColors.Surface.pageSecondary())
) {
val scrollState = rememberLazyListState(
if (state.bottomLayout) {
state.items.lastIndex
} else {
0
}
)
LazyColumn(
verticalArrangement = if (state.bottomLayout) Arrangement.Bottom else Arrangement.Top,
state = scrollState,
modifier = Modifier
.fillMaxSize()
.background(HorizonColors.Surface.pageSecondary()),
reverseLayout = state.bottomLayout,
.weight(1f)
.background(HorizonColors.Surface.pageSecondary())
.semantics {
isTraversalGroup = true
},
contentPadding = PaddingValues(top = 16.dp)
) {
if (state.replyState != null) {
stickyHeader { HorizonInboxReplyContent(state.replyState) }
}
items(state.items) {
itemsIndexed(state.items) { index, item ->
Column {
HorizonInboxDetailsItem(it)
if ((state.bottomLayout && it != state.items.firstOrNull()) || (!state.bottomLayout && it != state.items.lastOrNull())) {
HorizonInboxDetailsItem(
item,
Modifier.semantics(true) {}
)
if (item != state.items.lastOrNull()) {
HorizonDivider()
}
}
}
}
if (state.replyState != null) {
HorizonInboxReplyContent(state.replyState)
Copy link

@claude claude bot Dec 11, 2025

Choose a reason for hiding this comment

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

This is a significant behavioral change from the previous reverseLayout approach.

Concerns:

  1. reverseLayout = true with LazyColumn is the standard pattern for bottom-anchored lists (chat-style UIs) because it naturally handles scroll position when items are added/removed
  2. The new approach using Arrangement.Bottom with manual scroll state initialization may not handle dynamic content changes as gracefully
  3. When new messages are added via pagination or when the keyboard appears, the scroll position might jump unexpectedly

Recommendation:
If the goal is to fix accessibility traversal order, consider using reverseLayout with Modifier.semantics { isTraversalGroup = true; traversalIndex = ... } instead of completely changing the layout approach. This would maintain the proven scroll behavior while fixing accessibility.

Please ensure this has been thoroughly tested with:

  • Initial message load
  • Pagination (loading older messages)
  • Keyboard appearance/dismissal
  • Reply submission with immediate message addition

}
Copy link

@claude claude bot Dec 11, 2025

Choose a reason for hiding this comment

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

Moving the reply content from stickyHeader to outside the LazyColumn fundamentally changes the UX.

Previous behavior: Reply UI stayed visible at the top/bottom while scrolling through messages (sticky header behavior)

New behavior: Reply UI is now a separate component after the scrollable list

Questions:

  1. Was the sticky header causing accessibility issues?
  2. Is the reply UI supposed to always be visible, or should it scroll with content?
  3. How does this interact with the keyboard when replying?

If the sticky header was causing problems, consider using a Column with the reply UI and using Modifier.weight(1f) on the LazyColumn to achieve similar results while maintaining the always-visible reply behavior.

}
}

Expand Down Expand Up @@ -448,7 +466,6 @@ private fun HorizonInboxDetailsScreenPreview() {
onReplyTextValueChange = {},
onSendReply = {}
),
bottomLayout = true
)
)
}
6 changes: 3 additions & 3 deletions .../main/java/com/instructure/horizon/features/inbox/details/HorizonInboxDetailsViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ class HorizonInboxDetailsViewModel @Inject constructor(
attachment.toAttachmentUiState()
}
)
},
}.reversed(),
Copy link

@claude claude bot Dec 11, 2025

Choose a reason for hiding this comment

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

This reversal combined with the layout changes needs careful verification.

Question: With the new Arrangement.Bottom approach and this .reversed(), are messages now displayed in the correct chronological order?

Testing needed:

  1. Verify message order matches expected chronological ordering
  2. Test that pagination loads older messages in the correct position
  3. Ensure new message submission appears in the right place

The interaction between .reversed() here, the changed pagination order at line 395, and the new layout arrangement needs integration testing to ensure correctness.

replyState = getReplyState(),
bottomLayout = true
)
Expand Down Expand Up @@ -382,7 +382,7 @@ class HorizonInboxDetailsViewModel @Inject constructor(

_uiState.update {
it.copy(
items = conversation.messages.map { message ->
items = it.items + conversation.messages.map { message ->
HorizonInboxDetailsItem(
author = conversation.participants.firstOrNull { it.id == message.authorId }?.name.orEmpty(),
date = message.createdAt.toDate() ?: Date(),
Expand All @@ -392,7 +392,7 @@ class HorizonInboxDetailsViewModel @Inject constructor(
attachment.toAttachmentUiState()
}
)
} + it.items,
},
replyState = it.replyState?.copy(
replyTextValue = TextFieldValue(""),
isLoading = false,
Expand Down
9 changes: 8 additions & 1 deletion ...rizon/src/main/java/com/instructure/horizon/features/inbox/list/HorizonInboxListScreen.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ import androidx.compose.material3.pulltorefresh.PullToRefreshDefaults.Indicator
import androidx.compose.material3.pulltorefresh.rememberPullToRefreshState
import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.collectAsState
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
Expand All @@ -51,6 +50,8 @@ import androidx.compose.ui.Modifier
import androidx.compose.ui.draw.clip
import androidx.compose.ui.platform.LocalContext
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.semantics.semantics
import androidx.compose.ui.semantics.stateDescription
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.unit.dp
import androidx.core.content.ContextCompat
Expand Down Expand Up @@ -192,6 +193,7 @@ private fun LazyListScope.inboxHeader(
) {
IconButton(
iconRes = R.drawable.arrow_back,
contentDescription = stringResource(R.string.a11yNavigateBack),
size = IconButtonSize.NORMAL,
color = IconButtonColor.Inverse,
elevation = HorizonElevation.level4,
Expand Down Expand Up @@ -338,9 +340,14 @@ private fun InboxContentItem(
onClick: () -> Unit,
modifier: Modifier = Modifier
) {
val readState = stringResource(R.string.a11y_readInboxMessage)
val unreadState = stringResource(R.string.a11y_unreadInboxMessage)
Column(
modifier = modifier
.clickable { onClick() }
.semantics(true) {
stateDescription = if (item.isUnread) unreadState else readState
}
) {
Column(
modifier = Modifier
Expand Down
2 changes: 2 additions & 0 deletions libs/horizon/src/main/java/com/instructure/horizon/horizonui/molecules/ActionBottomSheet.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import androidx.compose.ui.Modifier
import androidx.compose.ui.draw.clip
import androidx.compose.ui.platform.LocalContext
import androidx.compose.ui.res.painterResource
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.text.style.TextAlign
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.unit.dp
Expand Down Expand Up @@ -86,6 +87,7 @@ fun ActionBottomSheet(
)
IconButton(
iconRes = R.drawable.close, color = IconButtonColor.Inverse,
contentDescription = stringResource(R.string.a11y_close),
onClick = {
localCoroutineScope.launch {
sheetState.hide()
Expand Down
33 changes: 29 additions & 4 deletions libs/horizon/src/main/java/com/instructure/horizon/horizonui/molecules/Tag.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ import androidx.compose.ui.graphics.Color
import androidx.compose.ui.platform.LocalContext
import androidx.compose.ui.res.painterResource
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.semantics.clearAndSetSemantics
import androidx.compose.ui.semantics.disabled
import androidx.compose.ui.semantics.hideFromAccessibility
import androidx.compose.ui.semantics.onClick
import androidx.compose.ui.semantics.semantics
import androidx.compose.ui.text.TextStyle
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.unit.Dp
Expand Down Expand Up @@ -78,7 +83,24 @@ fun Tag(
}
val badgePadding = if (dismissible && type == TagType.INLINE) PaddingValues(top = 8.dp, end = 8.dp) else PaddingValues()
val alphaModifier = if (enabled) modifier else modifier.alpha(0.5f)
Box(contentAlignment = Alignment.TopEnd, modifier = alphaModifier.padding(badgePadding)) {
val dismissText = stringResource(R.string.tag_dismiss)

Box(
contentAlignment = Alignment.TopEnd,
modifier = alphaModifier
.padding(badgePadding)
.semantics(mergeDescendants = true) {
if (!enabled) {
disabled()
}
if (dismissible && enabled) {
onClick(label = dismissText) {
onDismiss()
true
}
}
}
) {
Box(
modifier = Modifier
.background(
Expand All @@ -96,18 +118,21 @@ fun Tag(
Icon(
painter = painterResource(R.drawable.close),
tint = HorizonColors.Icon.default(),
contentDescription = stringResource(R.string.tag_dismiss),
contentDescription = null,
modifier = Modifier
.size(size.iconSize)
.clickable { onDismiss() })
.clickable(enabled = enabled) { onDismiss() }
.clearAndSetSemantics { hideFromAccessibility() }
)

}
}
}
if (dismissible && type == TagType.INLINE) {
Badge(
content = BadgeContent.Icon(
iconRes = R.drawable.close_small,
contentDescription = stringResource(R.string.tag_dismiss)
contentDescription = null
),
modifier = Modifier.offset(x = 8.dp, y = (-8).dp)
)
Expand Down
22 changes: 14 additions & 8 deletions libs/horizon/src/main/java/com/instructure/horizon/horizonui/molecules/filedrop/FileDrop.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,41 +118,46 @@ fun FileDrop(
sealed class FileDropItemState(
open val fileName: String,
open val actionIconRes: Int,
open val actionIconContentDescriptionRes: Int,
open val onActionClick: (() -> Unit)? = null,
open val onClick: (() -> Unit)? = null
) {
data class Success(
override val fileName: String,
override val onActionClick: (() -> Unit)? = null,
override val onClick: (() -> Unit)? = null,
override val actionIconRes: Int = R.drawable.delete
override val actionIconRes: Int = R.drawable.delete,
override val actionIconContentDescriptionRes: Int = R.string.a11y_delete
) :
FileDropItemState(fileName, actionIconRes)
FileDropItemState(fileName, actionIconRes, actionIconContentDescriptionRes)

data class InProgress(
override val fileName: String,
val progress: Float? = null,
override val onActionClick: (() -> Unit)? = null,
override val onClick: (() -> Unit)? = null,
override val actionIconRes: Int = R.drawable.close
override val actionIconRes: Int = R.drawable.close,
override val actionIconContentDescriptionRes: Int = R.string.a11y_cancel
) :
FileDropItemState(fileName, actionIconRes)
FileDropItemState(fileName, actionIconRes, actionIconContentDescriptionRes)

data class NoLongerEditable(
override val fileName: String,
override val onActionClick: (() -> Unit)? = null,
override val onClick: (() -> Unit)? = null,
override val actionIconRes: Int = R.drawable.download
override val actionIconRes: Int = R.drawable.download,
override val actionIconContentDescriptionRes: Int = R.string.a11y_download
) :
FileDropItemState(fileName, actionIconRes)
FileDropItemState(fileName, actionIconRes, actionIconContentDescriptionRes)

data class Error(
override val fileName: String,
override val onActionClick: (() -> Unit)? = null,
override val onClick: (() -> Unit)? = null,
override val actionIconRes: Int = R.drawable.refresh
override val actionIconRes: Int = R.drawable.refresh,
override val actionIconContentDescriptionRes: Int = R.string.a11y_retry
) :
FileDropItemState(fileName, actionIconRes)
FileDropItemState(fileName, actionIconRes, actionIconContentDescriptionRes)

}

Expand Down Expand Up @@ -217,6 +222,7 @@ fun FileDropItem(
state.onActionClick?.let {
IconButton(
iconRes = state.actionIconRes,
contentDescription = stringResource(state.actionIconContentDescriptionRes),
size = IconButtonSize.SMALL,
color = IconButtonColor.Inverse,
onClick = it
Expand Down
Loading
Loading