Skip to content

Commit 08636a6

Browse files
SaintPatrckclaude
andcommitted
Address code review feedback for LTR visual transformations
Simplifies password field visual transformations and adds comprehensive documentation to clarify usage patterns for future contributors. Changes: - Remove unnecessary CompoundVisualTransformation when password is obscured (bullets are directionally neutral) - Change ForceLtrVisualTransformation and CompoundVisualTransformation visibility from private to internal for testability - Add comprehensive KDoc explaining when to use LTR transformation: * Apply to technical identifiers (SSN, passport, license, etc.) * Do NOT apply to locale-dependent text (names, addresses, usernames) - Add usage examples to CompoundVisualTransformation - Create comprehensive test suites (39 test cases) to verify offset mapping correctness and edge case handling All tests pass and code compiles successfully. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 417726a commit 08636a6

File tree

6 files changed

+680
-19
lines changed

6 files changed

+680
-19
lines changed

ui/src/main/kotlin/com/bitwarden/ui/platform/components/field/BitwardenHiddenPasswordField.kt

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@ import com.bitwarden.ui.platform.base.util.nullableTestTag
1616
import com.bitwarden.ui.platform.components.field.color.bitwardenTextFieldColors
1717
import com.bitwarden.ui.platform.components.field.toolbar.BitwardenEmptyTextToolbar
1818
import com.bitwarden.ui.platform.components.model.CardStyle
19-
import com.bitwarden.ui.platform.components.util.compoundVisualTransformation
20-
import com.bitwarden.ui.platform.components.util.forceLtrVisualTransformation
2119
import com.bitwarden.ui.platform.theme.BitwardenTheme
2220

2321
/**
@@ -46,10 +44,7 @@ fun BitwardenHiddenPasswordField(
4644
label = label?.let { { Text(text = it) } },
4745
value = value,
4846
onValueChange = { },
49-
visualTransformation = compoundVisualTransformation(
50-
PasswordVisualTransformation(),
51-
forceLtrVisualTransformation(),
52-
),
47+
visualTransformation = PasswordVisualTransformation(),
5348
singleLine = true,
5449
enabled = false,
5550
readOnly = true,

ui/src/main/kotlin/com/bitwarden/ui/platform/components/field/BitwardenPasswordField.kt

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -143,10 +143,7 @@ fun BitwardenPasswordField(
143143
var lastTextValue by remember(value) { mutableStateOf(value = value) }
144144

145145
val visualTransformation = when {
146-
!showPassword -> compoundVisualTransformation(
147-
PasswordVisualTransformation(),
148-
forceLtrVisualTransformation(),
149-
)
146+
!showPassword -> PasswordVisualTransformation()
150147

151148
readOnly -> compoundVisualTransformation(
152149
nonLetterColorVisualTransformation(),

ui/src/main/kotlin/com/bitwarden/ui/platform/components/util/CompoundVisualTransformation.kt

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,43 @@ import androidx.compose.ui.text.input.VisualTransformation
1010
/**
1111
* A [VisualTransformation] that chains multiple other [VisualTransformation]s.
1212
*
13-
* This is useful for applying multiple transformations to a text field. The transformations
14-
* are applied in the order they are provided.
13+
* This is useful for applying multiple transformations to a text field. The
14+
* transformations are applied in the order they are provided, with each transformation's
15+
* output becoming the input for the next transformation.
16+
*
17+
* ## Example Usage
18+
*
19+
* Combining password masking with LTR direction enforcement:
20+
* ```kotlin
21+
* val visualTransformation = compoundVisualTransformation(
22+
* PasswordVisualTransformation(),
23+
* forceLtrVisualTransformation()
24+
* )
25+
* TextField(
26+
* value = password,
27+
* visualTransformation = visualTransformation
28+
* )
29+
* ```
30+
*
31+
* Combining color transformation with LTR for readonly fields:
32+
* ```kotlin
33+
* val visualTransformation = compoundVisualTransformation(
34+
* nonLetterColorVisualTransformation(),
35+
* forceLtrVisualTransformation()
36+
* )
37+
* ```
38+
*
39+
* ## Important Notes
40+
*
41+
* - Offset mapping is correctly composed through all transformations
42+
* - The order of transformations matters (first applied is first in the list)
43+
* - Use [compoundVisualTransformation] function for proper `remember` optimization
44+
*
45+
* @param transformations The visual transformations to apply in order
46+
* @see compoundVisualTransformation
47+
* @see forceLtrVisualTransformation
1548
*/
16-
private class CompoundVisualTransformation(
49+
internal class CompoundVisualTransformation(
1750
vararg val transformations: VisualTransformation,
1851
) : VisualTransformation {
1952
override fun filter(text: AnnotatedString): TransformedText {

ui/src/main/kotlin/com/bitwarden/ui/platform/components/util/ForceLtrVisualTransformation.kt

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,48 @@ import androidx.compose.ui.text.input.TransformedText
99
import androidx.compose.ui.text.input.VisualTransformation
1010

1111
// Unicode characters for forcing LTR direction
12-
private const val LRO = "\u202A"
13-
private const val PDF = "\u202C"
12+
internal const val LRO = "\u202A"
13+
internal const val PDF = "\u202C"
1414

1515
/**
1616
* A [VisualTransformation] that forces the output to have an LTR text direction.
1717
*
18-
* This is useful for password fields where the input should always be LTR, even when the rest of
19-
* the UI is RTL.
18+
* This transformation wraps text with Unicode directional control characters (LRO/PDF)
19+
* to ensure left-to-right rendering regardless of the UI's locale or text direction.
20+
*
21+
* ## When to Use
22+
*
23+
* Apply this transformation to fields containing **standardized, technical data** that is
24+
* always interpreted from left-to-right, regardless of locale:
25+
* - Passwords and sensitive authentication data
26+
* - Social Security Numbers (SSN)
27+
* - Driver's license numbers
28+
* - Passport numbers
29+
* - Payment card numbers
30+
* - Email addresses (technical format)
31+
* - Phone numbers (standardized format)
32+
* - URIs and technical identifiers
33+
*
34+
* ## When NOT to Use
35+
*
36+
* Do NOT apply this transformation to **locale-dependent text** that may legitimately
37+
* use RTL scripts:
38+
* - Personal names (may use Arabic, Hebrew, etc.)
39+
* - Company names
40+
* - Addresses
41+
* - Usernames (user choice)
42+
* - Notes and other free-form text
43+
*
44+
* ## Implementation Notes
45+
*
46+
* - Only applies LTR transformation when text is **visible**
47+
* - Do NOT use with obscured text (e.g., password bullets) as masked characters
48+
* are directionally neutral
49+
* - Can be composed with other transformations using [compoundVisualTransformation]
50+
*
51+
* @see compoundVisualTransformation
2052
*/
21-
private object ForceLtrVisualTransformation : VisualTransformation {
53+
internal object ForceLtrVisualTransformation : VisualTransformation {
2254
override fun filter(text: AnnotatedString): TransformedText {
2355
val forcedLtrText = buildAnnotatedString {
2456
append(LRO)
@@ -38,7 +70,7 @@ private object ForceLtrVisualTransformation : VisualTransformation {
3870
}
3971

4072
/**
41-
* Remembers a [ForceLtrVisualTransformation] for the given [transformations].
73+
* Remembers a [ForceLtrVisualTransformation] transformation.
4274
*
4375
* This is an optimization to avoid creating a new [ForceLtrVisualTransformation] on every
4476
* recomposition.

0 commit comments

Comments
 (0)