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.

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

Add accessibility tags #288

Merged
merged 1 commit into from
Dec 4, 2024
Merged

Add accessibility tags #288

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 16 additions & 0 deletions app/src/main/kotlin/com/ivanovsky/passnotes/presentation/core/compose/Accessibility.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package com.ivanovsky.passnotes.presentation.core.compose

import androidx.compose.runtime.Composable
import androidx.compose.runtime.remember
import androidx.compose.ui.Modifier
import androidx.compose.ui.semantics.contentDescription
import androidx.compose.ui.semantics.semantics

@Composable
fun contentDescription(description: String): Modifier {
return remember {
Modifier.semantics {
contentDescription = description
}
}
}
Comment on lines +1 to +16
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Add content descriptions to interactive UI elements

Several Compose UI components lack proper content descriptions for accessibility:

  • ImageButton in Buttons.kt explicitly sets contentDescription = null
  • Image components in HistoryHeaderCell.kt and InfoCell.kt have null content descriptions
  • Icons in TextField.kt have null content descriptions

The newly added contentDescription() modifier function is not being utilized yet. These missing descriptions could impact accessibility for users relying on screen readers.

🔗 Analysis chain

Verify usage patterns across the codebase

Let's ensure this new accessibility function is being used consistently across the codebase.

Let me search more specifically for UI components that might need accessibility descriptions and check if the new contentDescription function is being used.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for potential UI components that might need content descriptions
echo "Searching for Compose UI components that might need content descriptions..."
rg -t kotlin "Icon|Button|Image|TextField" app/src/main/kotlin

# Check current usage of the new contentDescription function
echo "Checking usage of contentDescription function..."
rg -t kotlin "contentDescription\(" app/src/main/kotlin

Length of output: 68267

Script:

#!/bin/bash
# Search for Compose UI components that commonly need content descriptions
echo "Searching for Compose UI components that need content descriptions..."
rg -t kotlin "Icon\(|Button\(|Image\(|IconButton\(" app/src/main/kotlin

# Check current usage of contentDescription modifier
echo "Checking usage of contentDescription modifier..."
rg -t kotlin "contentDescription\s*=" app/src/main/kotlin

Length of output: 5458

16 changes: 16 additions & 0 deletions app/src/main/kotlin/com/ivanovsky/passnotes/presentation/core/widget/MaterialEditText.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,11 @@ class MaterialEditText(
this.hint = hint
}

val description = params.getString(R.styleable.MaterialEditText_description)
if (description != null) {
binding.textInput.contentDescription = description
}

val isEyeButtonEnabled = params.getBoolean(
R.styleable.MaterialEditText_isEyeButtonEnabled,
false
Expand All @@ -157,10 +162,12 @@ class MaterialEditText(
isEyeButtonEnabled && isClearButtonEnabled -> {
throw IllegalStateException()
}

isEyeButtonEnabled -> {
actionButton = ActionButton.EYE
isTextVisible = false
}

isClearButtonEnabled -> {
actionButton = ActionButton.CLEAR
}
Expand Down Expand Up @@ -272,19 +279,23 @@ class MaterialEditText(
InputType.TYPE_CLASS_TEXT + InputType.TYPE_TEXT_VARIATION_PASSWORD
}
}

TextInputType.DIGITS -> {
if (isTextVisible) {
InputType.TYPE_CLASS_NUMBER
} else {
InputType.TYPE_CLASS_NUMBER + InputType.TYPE_NUMBER_VARIATION_PASSWORD
}
}

TextInputType.TEXT_CAP_SENTENCES -> {
InputType.TYPE_CLASS_TEXT + InputType.TYPE_TEXT_FLAG_CAP_SENTENCES
}

TextInputType.URL -> {
InputType.TYPE_CLASS_TEXT + InputType.TYPE_TEXT_VARIATION_URI
}

TextInputType.EMAIL -> {
InputType.TYPE_CLASS_TEXT + InputType.TYPE_TEXT_VARIATION_WEB_EMAIL_ADDRESS
}
Expand All @@ -300,11 +311,13 @@ class MaterialEditText(
binding.textInput.maxLines = 1
binding.textInput.isSingleLine = true
}

TextInputLines.MULTIPLE_LINES -> {
binding.textInput.isSingleLine = false
binding.textInput.minLines = 1
binding.textInput.maxLines = 25
}

else -> {}
}
setTextVisibleInternal(isTextVisible)
Expand All @@ -316,9 +329,11 @@ class MaterialEditText(
ImeOptions.ACTION_DONE -> {
binding.textInput.imeOptions = EditorInfo.IME_ACTION_DONE
}

ImeOptions.ACTION_NEXT -> {
binding.textInput.imeOptions = EditorInfo.IME_ACTION_NEXT
}

else -> {}
}
}
Expand Down Expand Up @@ -353,6 +368,7 @@ class MaterialEditText(
}
binding.editTextActionButton.setImageResource(getEyeIcon(isTextVisible))
}

ActionButton.CLEAR -> {
binding.editTextActionButton.setImageResource(R.drawable.ic_close_24dp)
}
Expand Down
5 changes: 5 additions & 0 deletions app/src/main/kotlin/com/ivanovsky/passnotes/presentation/serverLogin/ServerLoginScreen.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.res.dimensionResource
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.semantics.contentDescription
import androidx.compose.ui.tooling.preview.Preview
import androidx.lifecycle.compose.collectAsStateWithLifecycle
import com.ivanovsky.passnotes.R
Expand All @@ -27,6 +28,7 @@ import com.ivanovsky.passnotes.presentation.core.compose.PrimaryTextStyle
import com.ivanovsky.passnotes.presentation.core.compose.ProgressIndicator
import com.ivanovsky.passnotes.presentation.core.compose.ThemedScreenPreview
import com.ivanovsky.passnotes.presentation.core.compose.cells.ui.InfoCell
import com.ivanovsky.passnotes.presentation.core.compose.contentDescription
import com.ivanovsky.passnotes.presentation.core.compose.model.InputType
import com.ivanovsky.passnotes.presentation.serverLogin.model.LoginType
import com.ivanovsky.passnotes.presentation.serverLogin.model.ServerLoginIntent.OnIgnoreSslValidationStateChanged
Expand Down Expand Up @@ -225,6 +227,7 @@ private fun UrlTextField(
end = dimensionResource(R.dimen.element_margin),
top = dimensionResource(R.dimen.element_margin)
)
.then(contentDescription(stringResource(R.string.url)))
)
}

Expand Down Expand Up @@ -302,6 +305,7 @@ private fun UsernameTextField(
end = dimensionResource(R.dimen.element_margin),
top = dimensionResource(R.dimen.element_margin)
)
.then(contentDescription(stringResource(R.string.username)))
)
}

Expand Down Expand Up @@ -332,6 +336,7 @@ private fun PasswordTextField(
end = dimensionResource(R.dimen.element_margin),
top = dimensionResource(R.dimen.element_margin)
)
.then(contentDescription(stringResource(R.string.password)))
)
}

Expand Down
3 changes: 3 additions & 0 deletions app/src/main/res/layout/dialog_change_password.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
android:layout_marginStart="@dimen/element_margin"
android:layout_marginTop="@dimen/group_margin"
android:layout_marginEnd="@dimen/element_margin"
app:description="@string/password"
app:hint="@string/password"
app:isEyeButtonEnabled="true"
app:layout_constraintBottom_toTopOf="@id/newPassword"
Expand All @@ -52,6 +53,7 @@
android:layout_width="0dp"
android:layout_height="wrap_content"
android:layout_marginTop="@dimen/element_margin"
app:description="@string/new_password"
app:hint="@string/new_password"
app:isEyeButtonEnabled="true"
app:layout_constraintBottom_toTopOf="@id/confirmation"
Expand All @@ -68,6 +70,7 @@
android:layout_width="0dp"
android:layout_height="wrap_content"
android:layout_marginTop="@dimen/element_margin"
app:description="@string/confirm_password"
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance password confirmation accessibility and error announcements

The confirmation field needs clear distinction and proper error state handling for accessibility.

  1. Enhance the description:
-app:description="@string/confirm_password"
+app:description="@string/confirm_password_input_description"
+android:importantForAccessibility="yes"
+android:accessibilityLiveRegion="polite"
  1. Add to strings.xml:
<string name="confirm_password_input_description">Re-enter your new password for confirmation</string>
  1. Ensure error states are properly announced:
+android:accessibilityLiveRegion="assertive"
+app:errorAccessibilityAnnouncement="@{viewModel.confirmationError}"

This will ensure that any password mismatch errors are properly announced to screen reader users.

app:hint="@string/confirm_password"
app:isEyeButtonEnabled="true"
app:layout_constraintBottom_toTopOf="@id/applyButton"
Expand Down
5 changes: 4 additions & 1 deletion app/src/main/res/layout/group_editor_fragment.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@
android:layout_width="match_parent"
android:layout_height="match_parent"
app:screenState="@{viewModel.screenState}"
app:screenStateHandler="@{viewModel.screenStateHandler}" />
app:screenStateHandler="@{viewModel.screenStateHandler}"
tools:layout_editor_absoluteX="0dp"
tools:layout_editor_absoluteY="0dp" />

<com.ivanovsky.passnotes.presentation.core.widget.MaterialEditText
android:id="@+id/title"
Expand All @@ -39,6 +41,7 @@
android:layout_marginLeft="@dimen/element_margin"
android:layout_marginTop="@dimen/group_margin"
android:layout_marginRight="@dimen/element_margin"
app:description="@string/title"
app:hint="@string/title"
app:layout_constraintTop_toBottomOf="@id/errorPanelView"
app:screenState="@{viewModel.screenState}"
Expand Down
1 change: 1 addition & 0 deletions app/src/main/res/layout/groups_fragment.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
android:layout_marginStart="@dimen/element_margin"
android:layout_marginTop="@dimen/group_margin"
android:layout_marginEnd="@dimen/element_margin"
app:description="@string/query"
app:hint="@string/query"
app:isClearButtonEnabled="true"
app:layout_constraintTop_toBottomOf="@id/navigationPanelLayout"
Expand Down
5 changes: 4 additions & 1 deletion app/src/main/res/layout/new_database_fragment.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@
app:layout_constraintEnd_toStartOf="@id/guidelineEnd"
app:layout_constraintTop_toTopOf="@id/addTemplatesCheckBox"
app:srcCompat="@drawable/ic_info_24dp"
app:tint="?attr/kpIconPrimaryColor"/>
app:tint="?attr/kpIconPrimaryColor" />

<com.ivanovsky.passnotes.presentation.core.widget.MaterialEditText
android:id="@+id/fileExtension"
Expand All @@ -151,6 +151,7 @@
android:layout_width="0dp"
android:layout_height="wrap_content"
android:layout_marginTop="@dimen/element_margin"
app:description="@string/file_name"
app:hint="@string/file_name"
app:isEnabled="@{viewModel.isFilenameEnabled}"
app:layout_constraintEnd_toStartOf="@id/fileExtension"
Expand All @@ -165,6 +166,7 @@
android:layout_width="0dp"
android:layout_height="wrap_content"
android:layout_marginTop="@dimen/element_margin"
app:description="@string/password"
app:hint="@string/password"
app:isEyeButtonEnabled="true"
app:layout_constraintEnd_toStartOf="@id/guidelineEnd"
Expand All @@ -179,6 +181,7 @@
android:layout_width="0dp"
android:layout_height="wrap_content"
android:layout_marginTop="@dimen/element_margin"
app:description="@string/confirm_password"
app:hint="@string/confirm"
app:isEyeButtonEnabled="true"
app:layout_constraintEnd_toStartOf="@id/guidelineEnd"
Expand Down
1 change: 1 addition & 0 deletions app/src/main/res/layout/password_generator_fragment.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
android:layout_height="wrap_content"
android:layout_marginHorizontal="@dimen/element_margin"
android:layout_marginTop="@dimen/element_margin"
app:description="@string/generated_password"
app:hint="@string/generated_password"
app:isEyeButtonEnabled="true"
app:layout_constraintTop_toTopOf="parent" />
Expand Down
3 changes: 2 additions & 1 deletion app/src/main/res/layout/widget_material_edit_text.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@

<com.google.android.material.textfield.TextInputEditText
android:id="@+id/textInput"
android:saveEnabled="false"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:saveEnabled="false"
android:textColor="?attr/kpPrimaryTextColor"
android:textSize="@dimen/material_edit_text_primary_text_size" />

Expand All @@ -30,6 +30,7 @@
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginTop="@dimen/small_margin"
android:contentDescription="@string/eye_icon"
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider dynamic content description for visibility toggle

The static content description "@string/eye_icon" might not be descriptive enough. Consider making it dynamic based on the visibility state (e.g., "Show password" / "Hide password").

This should be handled in the corresponding Kotlin code rather than the layout XML.

android:focusable="false"
android:focusableInTouchMode="false"
Comment on lines 34 to 35
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Review focusability settings for accessibility

Setting focusable="false" and focusableInTouchMode="false" on the ImageButton might prevent screen reader users from accessing the visibility toggle functionality.

Consider removing these attributes to ensure the button is accessible via screen readers and keyboard navigation.

-        android:focusable="false"
-        android:focusableInTouchMode="false"
+        android:focusable="true"
+        android:importantForAccessibility="yes"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
android:focusable="false"
android:focusableInTouchMode="false"
android:focusable="true"
android:importantForAccessibility="yes"

android:visibility="gone"
Expand Down
3 changes: 2 additions & 1 deletion app/src/main/res/layout/widget_unlock_view.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
android:layout_height="wrap_content"
android:layout_marginTop="@dimen/half_margin"
android:layout_marginEnd="@dimen/half_margin"
app:description="@string/password"
app:hint="@string/password"
app:isEyeButtonEnabled="true"
app:layout_constraintEnd_toStartOf="@id/unlockButton"
Expand All @@ -34,9 +35,9 @@
android:layout_width="wrap_content"
android:layout_height="0dp"
android:layout_marginTop="@dimen/small_margin"
android:contentDescription="@string/unlock_button"
android:foreground="?attr/selectableItemBackground"
android:minWidth="@dimen/unlock_button_size"
android:contentDescription="unlockButton"
app:layout_constraintBottom_toBottomOf="@id/password"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintTop_toTopOf="@id/password" />
Expand Down
1 change: 1 addition & 0 deletions app/src/main/res/values/attrs.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,6 @@
<attr name="hint" format="string" />
<attr name="isEyeButtonEnabled" format="boolean" />
<attr name="isClearButtonEnabled" format="boolean" />
<attr name="description" format="string" />
</declare-styleable>
</resources>
5 changes: 5 additions & 0 deletions app/src/main/res/values/strings.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@
<string name="synchronize_with_source">Synchronize with source</string>
<string name="details">Details</string>

<!-- Accessibility descriptions -->
<string name="url">URL</string>
<string name="unlock_button">Unlock button</string>
<string name="eye_icon">Eye icon</string>

<!-- Lock notification -->
<string name="lock_notification_channel">Quick Lock</string>
<string name="lock_notification_channel_description">Button to lock opened database</string>
Expand Down
Loading