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

Issue #558: Improve on accessibility for Spotlight #597

Closed
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,15 @@ import androidx.compose.material.Button
import androidx.compose.material.ExperimentalMaterialApi
import androidx.compose.material.Text
import androidx.compose.runtime.Composable
import androidx.compose.runtime.collectAsState
import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.input.pointer.pointerInput
import androidx.compose.ui.semantics.CollectionInfo
import androidx.compose.ui.semantics.CustomAccessibilityAction
import androidx.compose.ui.semantics.collectionInfo
import androidx.compose.ui.semantics.customActions
import androidx.compose.ui.semantics.semantics
import androidx.compose.ui.unit.dp
import com.bumble.appyx.components.spotlight.Spotlight
import com.bumble.appyx.components.spotlight.SpotlightModel
Expand Down Expand Up @@ -69,11 +75,12 @@ fun SpotlightExperiment(
Target.Child6,
Target.Child7,
)
val spotlightModel = SpotlightModel(
items = items,
savedStateMap = null
)
val spotlight = Spotlight(
model = SpotlightModel(
items = items,
savedStateMap = null
),
model = spotlightModel,
motionController = motionController,
gestureFactory = { SpotlightSlider.Gestures(it, orientation, reverseOrientation) },
animationSpec = spring(stiffness = Spring.StiffnessVeryLow / 4),
Expand All @@ -93,6 +100,7 @@ fun SpotlightExperiment(
) {
SpotlightUi(
spotlight = spotlight,
model = spotlightModel,
modifier = Modifier.weight(0.9f)
)

Expand Down Expand Up @@ -130,6 +138,7 @@ fun SpotlightExperiment(
@Composable
fun <InteractionTarget : Any> SpotlightUi(
spotlight: Spotlight<InteractionTarget>,
model: SpotlightModel<InteractionTarget>,
modifier: Modifier = Modifier,
color: Color = Color.Unspecified
) {
Expand All @@ -140,15 +149,47 @@ fun <InteractionTarget : Any> SpotlightUi(
.padding(
horizontal = 64.dp,
vertical = 12.dp
),
)
.semantics {
collectionInfo = CollectionInfo(1, spotlight.elements.value.all.size)
},
element = { elementUiModel ->
val elementIndex = spotlight.elements.value.all.indexOf(elementUiModel.element)
val state = model.output.collectAsState()
val activeIndex = state.value.currentTargetState.activeIndex.toInt()
Element(
color = color,
elementUiModel = elementUiModel,
contentDescription =
"${SPOTLIGHT_EXPERIMENT_TEST_HELPER}_${elementUiModel.element.id}",
modifier = Modifier
.fillMaxSize()
.semantics(mergeDescendants = true) {
customActions =
if (elementIndex == activeIndex) {
listOfNotNull(
CustomAccessibilityAction("Next") {
Copy link
Collaborator

@CherryPerry CherryPerry Sep 15, 2023

Choose a reason for hiding this comment

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

Not localised.

Also can you use non-custom actions that are used in androidx.compose.HorizontalPager?

https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/pager/Pager.kt;l=726?q=androidx.compose.foundation.pager&sq=&ss=androidxpageUp { } & pageDown { } on SampleChildren

https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/pager/LazyLayoutSemanticState.kt;l=44CollectionInfo (you are already using it)

IDK how to make it focus the scrolled page (on your video it does not focus on it). probably somewhere in sources too. Please compare TalkBack behaviour to the default HorizontalPager.

Potentially SpotlightModel should be able to provide Modifier.semantics to both parent and each children.

spotlight.next()
true
}
.takeIf { elementIndex != spotlight.elements.value.all.size - 1 },
CustomAccessibilityAction("Previous") {
spotlight.previous()
true
}
.takeIf { elementIndex != 0 }
)
} else {
listOf(
CustomAccessibilityAction("Select") {
if (elementIndex < activeIndex) {
spotlight.previous()
} else {
spotlight.next()
}
true
}
)
}
}
.pointerInput(elementUiModel.element.id) {
detectDragGestures(
onDrag = { change, dragAmount ->
Expand Down