Skip to content

Forms : Read only fields re-design #346

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
merged 6 commits into from
Mar 4, 2024
Merged

Forms : Read only fields re-design #346

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ae923fa
added new read-only field design
kaushikrw Mar 1, 2024
e65c73e
fixed tests
kaushikrw Mar 2, 2024
4ebd7b5
updated doc
kaushikrw Mar 2, 2024
e4ef3fd
removed unused imports
kaushikrw Mar 2, 2024
79d06ee
remove unused imports
kaushikrw Mar 2, 2024
792e8fe
undo space
kaushikrw Mar 2, 2024
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
6 changes: 4 additions & 2 deletions toolkit/featureforms/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ FeatureForm(

#### Text Fields
- Outline color - `MaterialTheme.colorScheme.outline`
- Label TextStyle - `MaterialTheme.typography.bodyMedium`
- Label TextStyle - `MaterialTheme.typography.bodySmall`
- Input TextStyle - `MaterialTheme.typography.bodyLarge`
- SupportingText TextStyle - `MaterialTheme.typography.bodySmall`
- Error color - `MaterialTheme.colorScheme.error`
Expand All @@ -121,6 +121,8 @@ FeatureForm(
- Description TextStyle - `MaterialTheme.typography.bodySmall`

#### Read-Only Fields
Alpha values for read only fields cannot be customized at this time.
- Label TextStyle - `MaterialTheme.typography.bodyMedium`
- Input TextStyle - `MaterialTheme.typography.bodyLarge`
- SupportingText TextStyle - `MaterialTheme.typography.bodySmall`

More information on the material 3 specs [here](https://m3.material.io/components/text-fields/specs#e4964192-72ad-414f-85b4-4b4357abb83c)
15 changes: 4 additions & 11 deletions ...atureforms/src/androidTest/java/com/arcgismaps/toolkit/featureforms/FormTextFieldTests.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,7 @@ class FormTextFieldTests {
val label = composeTestRule.onNodeWithContentDescription(labelSemanticLabel)
label.assertIsDisplayed()

val supportingText = composeTestRule.onNode(hasContentDescription(supportingTextSemanticLabel), useUnmergedTree = true)
supportingText.assertExists()
composeTestRule.onNode(hasContentDescription(supportingTextSemanticLabel), useUnmergedTree = true).assertDoesNotExist()
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍 thanks for pointing this out in the PR comment.

}

/**
Expand All @@ -134,9 +133,7 @@ class FormTextFieldTests {
val label = composeTestRule.onNodeWithContentDescription(labelSemanticLabel)
label.assertIsDisplayed()

val supportingText = composeTestRule.onNode(hasContentDescription(supportingTextSemanticLabel), useUnmergedTree = true)
supportingText.assertExists()
assertEquals(field.description, supportingText.getTextString())
composeTestRule.onNode(hasContentDescription(supportingTextSemanticLabel), useUnmergedTree = true).assertDoesNotExist()
}

/**
Expand All @@ -155,9 +152,7 @@ class FormTextFieldTests {
val label = composeTestRule.onNodeWithContentDescription(labelSemanticLabel)
label.assertIsDisplayed()

val supportingText = composeTestRule.onNode(hasContentDescription(supportingTextSemanticLabel), useUnmergedTree = true)
supportingText.assertExists()
assertEquals(field.description, supportingText.getTextString())
composeTestRule.onNode(hasContentDescription(supportingTextSemanticLabel), useUnmergedTree = true).assertDoesNotExist()

val charCountNode =
composeTestRule.onNode(hasContentDescription(charCountSemanticLabel), useUnmergedTree = true)
Expand Down Expand Up @@ -185,9 +180,7 @@ class FormTextFieldTests {
val label = composeTestRule.onNodeWithContentDescription(labelSemanticLabel)
label.assertIsDisplayed()

val supportingText = composeTestRule.onNode(hasContentDescription(supportingTextSemanticLabel), useUnmergedTree = true)
supportingText.assertExists()
assertEquals(field.description, supportingText.getTextString())
composeTestRule.onNode(hasContentDescription(supportingTextSemanticLabel), useUnmergedTree = true).assertDoesNotExist()

outlinedTextField.performImeAction()
outlinedTextField.assertIsNotFocused()
Expand Down
1 change: 1 addition & 0 deletions toolkit/featureforms/src/main/java/com/arcgismaps/toolkit/featureforms/FeatureForm.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ private fun FeatureFormBody(
// other form elements are not created
}
}
Divider()
Copy link
Collaborator

Choose a reason for hiding this comment

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

It might be good to only surround read only fields with dividers, but it is hard to tell without looking at it and it would require a small amount of brain surgery here to see it. If we get feedback, let's look at it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree, lets get some feedback and iterate.

}
}
}
Expand Down
Loading