Skip to content

[CLX-3694][Horizon] Accessibility fixes #3454

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
andrasmaczak merged 2 commits into from
Dec 18, 2025
Merged

[CLX-3694][Horizon] Accessibility fixes #3454

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
012a9f1
accessibility fixes
andrasmaczak Dec 17, 2025
3c50c00
PR fix
andrasmaczak Dec 18, 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
13 changes: 10 additions & 3 deletions libs/horizon/src/main/java/com/instructure/horizon/features/dashboard/DashboardScreen.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,11 @@ fun DashboardScreen(uiState: DashboardUiState, mainNavController: NavHostControl
refreshStateFlow
)
HorizonSpace(SpaceSize.SPACE_16)
NumericWidgetRow(shouldRefresh, refreshStateFlow, homeNavController)
NumericWidgetRow(
shouldRefresh,
refreshStateFlow,
homeNavController
)
DashboardSkillHighlightsWidget(
homeNavController,
shouldRefresh,
Expand Down Expand Up @@ -280,9 +284,12 @@ private fun HomeScreenTopBar(uiState: DashboardUiState, mainNavController: NavCo
private fun NumericWidgetRow(
shouldRefresh: Boolean,
refreshStateFlow: MutableStateFlow<List<Boolean>>,
homeNavController: NavHostController
homeNavController: NavHostController,
modifier: Modifier = Modifier
) {
BoxWithConstraints {
BoxWithConstraints(
modifier = modifier
) {
val pageCount = 3
val pagerState = rememberPagerState{ pageCount }
if (this.isWideLayout) {
Expand Down
6 changes: 5 additions & 1 deletion ...on/src/main/java/com/instructure/horizon/features/dashboard/widget/DashboardWidgetCard.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.ColumnScope
import androidx.compose.foundation.layout.IntrinsicSize
import androidx.compose.foundation.layout.Row
import androidx.compose.foundation.layout.Spacer
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.layout.size
Expand All @@ -41,6 +40,7 @@ import androidx.compose.ui.res.painterResource
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.semantics.clearAndSetSemantics
import androidx.compose.ui.semantics.contentDescription
import androidx.compose.ui.semantics.semantics
import androidx.compose.ui.text.style.TextOverflow
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.unit.dp
Expand Down Expand Up @@ -82,6 +82,10 @@ fun DashboardWidgetCard(
contentDescription =
context.getString(R.string.a11y_dashboardWidgetLoadingContentDescription, title)
}
}.conditional(!isLoading) {
semantics {
contentDescription = ""
Copy link

@claude claude bot Dec 17, 2025

Choose a reason for hiding this comment

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

Setting contentDescription = "" when not loading may interfere with accessibility. An empty content description can sometimes be problematic for screen readers. Consider using clearAndSetSemantics { } instead if you want to clear all semantics, or verify that an empty string is the intended behavior for this use case.

If the goal is to ensure child elements provide their own content descriptions, this approach is correct. Just wanted to flag for verification.

}
},
onClick
) {
Expand Down
10 changes: 8 additions & 2 deletions ...cture/horizon/features/dashboard/widget/announcement/DashboardAnnouncementBannerWidget.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.collectAsState
import androidx.compose.runtime.getValue
import androidx.compose.ui.Modifier
import androidx.compose.ui.res.stringResource
import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel
import androidx.navigation.NavHostController
Expand All @@ -38,7 +39,8 @@ fun DashboardAnnouncementBannerWidget(
mainNavController: NavHostController,
homeNavController: NavHostController,
shouldRefresh: Boolean,
refreshState: MutableStateFlow<List<Boolean>>
refreshState: MutableStateFlow<List<Boolean>>,
modifier: Modifier = Modifier,
) {
val viewModel = hiltViewModel<DashboardAnnouncementBannerViewModel>()
val state by viewModel.uiState.collectAsState()
Expand All @@ -53,7 +55,7 @@ fun DashboardAnnouncementBannerWidget(
}

if (state.state != DashboardItemState.SUCCESS || state.cardState.items.isNotEmpty()) {
DashboardAnnouncementBannerSection(state, mainNavController, homeNavController)
DashboardAnnouncementBannerSection(state, mainNavController, homeNavController, modifier)
}
}

Expand All @@ -62,13 +64,15 @@ fun DashboardAnnouncementBannerSection(
state: DashboardAnnouncementBannerUiState,
mainNavController: NavHostController,
homeNavController: NavHostController,
modifier: Modifier = Modifier,
) {
when (state.state) {
DashboardItemState.LOADING -> {
DashboardPaginatedWidgetCard(
DashboardPaginatedWidgetCardState.Loading,
mainNavController,
homeNavController,
modifier
)
}
DashboardItemState.ERROR -> {
Expand All @@ -79,13 +83,15 @@ fun DashboardAnnouncementBannerSection(
false,
DashboardWidgetPageState.Empty,
{ state.onRefresh {} },
modifier
)
}
DashboardItemState.SUCCESS -> {
DashboardPaginatedWidgetCard(
state.cardState,
mainNavController,
homeNavController,
modifier
)
}
}
Expand Down
24 changes: 16 additions & 8 deletions ...n/java/com/instructure/horizon/features/dashboard/widget/course/DashboardCourseSection.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.layout.onGloballyPositioned
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.semantics.contentDescription
import androidx.compose.ui.semantics.semantics
import androidx.compose.ui.unit.dp
import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel
import androidx.navigation.NavGraph.Companion.findStartDestination
Expand Down Expand Up @@ -70,7 +72,8 @@ fun DashboardCourseSection(
mainNavController: NavHostController,
homeNavController: NavHostController,
shouldRefresh: Boolean,
refreshState: MutableStateFlow<List<Boolean>>
refreshState: MutableStateFlow<List<Boolean>>,
modifier: Modifier = Modifier,
) {
val viewModel = hiltViewModel<DashboardCourseViewModel>()
val state by viewModel.uiState.collectAsState()
Expand All @@ -84,29 +87,30 @@ fun DashboardCourseSection(
}
}

DashboardCourseSection(state, mainNavController, homeNavController)
DashboardCourseSection(state, mainNavController, homeNavController, modifier)
}

@Composable
fun DashboardCourseSection(
state: DashboardCourseUiState,
mainNavController: NavHostController,
homeNavController: NavHostController
homeNavController: NavHostController,
modifier: Modifier = Modifier,
) {
when(state.state) {
DashboardItemState.LOADING -> {
DashboardCourseCardContent(
DashboardCourseCardState.Loading,
{ handleClickAction(it, mainNavController, homeNavController) },
true,
modifier = Modifier.padding(horizontal = 24.dp)
modifier = modifier.padding(horizontal = 24.dp)
)
}
DashboardItemState.ERROR -> {
DashboardCourseCardError({state.onRefresh {} }, Modifier.padding(horizontal = 24.dp))
DashboardCourseCardError({state.onRefresh {} }, modifier.padding(horizontal = 24.dp))
}
DashboardItemState.SUCCESS -> {
DashboardCourseSectionContent(state, mainNavController, homeNavController)
DashboardCourseSectionContent(state, mainNavController, homeNavController, modifier)
}
}
}
Expand All @@ -115,13 +119,15 @@ fun DashboardCourseSection(
private fun DashboardCourseSectionContent(
state: DashboardCourseUiState,
mainNavController: NavHostController,
homeNavController: NavHostController
homeNavController: NavHostController,
modifier: Modifier = Modifier,
) {
// Display 4 cards at most
val pagerState = rememberPagerState { min(4, state.courses.size) }

Column(
horizontalAlignment = Alignment.CenterHorizontally,
modifier = modifier
) {
if (state.programs.items.isNotEmpty()) {
DashboardPaginatedWidgetCard(
Expand Down Expand Up @@ -151,6 +157,9 @@ private fun DashboardCourseSectionContent(
val cardHeight = coordinates.size.height
if (cardHeight > maxCardHeight) { maxCardHeight = cardHeight }
}
.semantics {
contentDescription = ""
Copy link

@claude claude bot Dec 17, 2025

Choose a reason for hiding this comment

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

Same concern as with DashboardWidgetCard - setting contentDescription = "" may not be the best approach. Verify this is intentional and doesn't hide important information from screen readers.

Additionally, this semantic block is only applied in the pager context when page < pagerState.pageCount. Consider whether this semantic clearing should also apply to other cases or if there's a reason for this conditional application.

}
)
}
else -> {
Expand All @@ -164,7 +173,6 @@ private fun DashboardCourseSectionContent(
}
}
}

}

Button(
Expand Down
21 changes: 19 additions & 2 deletions ...main/java/com/instructure/horizon/features/moduleitemsequence/ModuleItemSequenceScreen.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ import androidx.compose.ui.layout.onGloballyPositioned
import androidx.compose.ui.platform.LocalContext
import androidx.compose.ui.platform.LocalDensity
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.semantics.hideFromAccessibility
import androidx.compose.ui.semantics.semantics
import androidx.compose.ui.text.style.TextAlign
import androidx.compose.ui.text.style.TextOverflow
import androidx.compose.ui.tooling.preview.Preview
Expand Down Expand Up @@ -326,7 +328,12 @@ private fun ModuleHeaderContainer(
) {
Column(modifier = modifier, horizontalAlignment = Alignment.CenterHorizontally) {
Row {
IconButton(iconRes = R.drawable.arrow_back, color = IconButtonColor.Institution, onClick = onBackPressed)
IconButton(
iconRes = R.drawable.arrow_back,
contentDescription = stringResource(R.string.a11yNavigateBack),
color = IconButtonColor.Institution,
onClick = onBackPressed
)
Column(
modifier = Modifier
.weight(1f)
Expand All @@ -350,6 +357,7 @@ private fun ModuleHeaderContainer(
}
IconButton(
iconRes = R.drawable.list_alt,
contentDescription = stringResource(R.string.myProgress),
color = IconButtonColor.Institution,
onClick = uiState.onProgressClick
)
Expand All @@ -365,7 +373,16 @@ private fun ModuleHeaderContainer(
if (index < uiState.currentItem?.detailTags?.lastIndex.orDefault()) listOf(item, "|") else listOf(item)
}
separatedFlowRowItems.forEach {
Text(text = it, style = HorizonTypography.p2, color = HorizonColors.Text.surfaceColored())
Text(
text = it,
style = HorizonTypography.p2,
color = HorizonColors.Text.surfaceColored(),
modifier = Modifier.semantics {
if (it == "|") {
hideFromAccessibility()
}
}
)
}
}
}
Expand Down
1 change: 1 addition & 0 deletions ...zon/features/moduleitemsequence/content/assignment/attempts/AttemptSelectorBottomSheet.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ fun AttemptSelectorBottomSheet(
Box(modifier = Modifier.padding(start = 24.dp, end = 24.dp, top = 24.dp)) {
IconButton(
iconRes = R.drawable.close,
contentDescription = stringResource(R.string.a11y_close),
color = IconButtonColor.Inverse,
modifier = Modifier
.align(Alignment.CenterEnd)
Expand Down
1 change: 1 addition & 0 deletions ...ructure/horizon/features/moduleitemsequence/content/assignment/comments/CommentsDialog.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ fun CommentsDialog(
Box(modifier = Modifier.padding(start = 24.dp, end = 24.dp, top = 24.dp)) {
IconButton(
iconRes = R.drawable.close,
contentDescription = stringResource(R.string.a11y_close),
color = IconButtonColor.Inverse,
modifier = Modifier
.align(Alignment.CenterEnd)
Expand Down
Loading
Loading