ADFA-3375: Support TextInputLayout and TextInputEditText in preview#1312
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 Walkthrough
WalkthroughAdds Material Components Material Text Field Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/views/StructureView.kt (1)
148-163:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTextInput type icon/name mapping is currently bypassed.
On Line 156, using
view.javaClass.superclass.simpleNameprevents the newimgMapentries forTextInputLayoutandTextInputEditTextfrom being hit, so these nodes render as fallback/incorrect types in the structure list.Proposed fix
- } else { - val viewSimpleName = view.javaClass.superclass.simpleName + } else { + val viewSimpleName = + when (view) { + is TextInputLayout, is TextInputEditText -> view.javaClass.simpleName + else -> view.javaClass.superclass.simpleName + } var imageResource = imgMap[viewSimpleName] if (imageResource == null) { imageResource = imgMap["_unknown"] } icon.setImageResource(imageResource!!) viewName.text = viewSimpleName }Also applies to: 404-405
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/views/StructureView.kt` around lines 148 - 163, The view type detection incorrectly uses view.javaClass.superclass.simpleName (in the else branch where viewSimpleName is set), which bypasses mappings for TextInputLayout/TextInputEditText; change the logic to use view.javaClass.simpleName to look up imgMap (falling back to imgMap["_unknown"] as before) and update any related display text (viewName.text) to use that actual simpleName so TextInput* entries resolve correctly; ensure this change touches the variables viewSimpleName and imageResource lookup in StructureView (keeping the existing LinearLayout special-case intact).
♻️ Duplicate comments (1)
layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/palette/text/TextInputLayoutDesign.kt (1)
14-37:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPotential double-drawing of stroke in blueprint mode.
This class has the same draw/dispatchDraw override pattern as
TextInputEditTextDesign, which can result in double-drawing the dashed stroke when bothisBlueprintanddrawStrokeEnabledare true. See the comment on TextInputEditTextDesign.kt for details and proposed fix.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/palette/text/TextInputLayoutDesign.kt` around lines 14 - 37, The stroke is being drawn in both dispatchDraw and draw when isBlueprint and drawStrokeEnabled are true; centralize drawing to avoid double-draw by removing the stroke call from draw and making dispatchDraw responsible for all stroke drawing. Change draw(Canvas) to always call super.draw(canvas) (remove the Utils.drawDashPathStroke call there), and update dispatchDraw(Canvas) to draw the dashed stroke when (drawStrokeEnabled || isBlueprint) using Utils.drawDashPathStroke(this, canvas, if (isBlueprint) Constants.BLUEPRINT_DASH_COLOR else Constants.DESIGN_DASH_COLOR). Keep setStrokeEnabled, drawStrokeEnabled and isBlueprint logic unchanged.
🧹 Nitpick comments (3)
layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/callers/text/TextInputLayoutCaller.kt (1)
24-37: 💤 Low valuePrefer Kotlin's
toBoolean()over Java'sparseBoolean().Lines 26, 31, and 36 use
java.lang.Boolean.parseBoolean(value). In Kotlin, the idiomatic approach is to use theString.toBoolean()extension function.♻️ Proposed Kotlin idiom
`@JvmStatic` fun setHintEnabled(target: View, value: String, context: Context) { - (target as TextInputLayout).isHintEnabled = java.lang.Boolean.parseBoolean(value) + (target as TextInputLayout).isHintEnabled = value.toBoolean() } `@JvmStatic` fun setErrorEnabled(target: View, value: String, context: Context) { - (target as TextInputLayout).isErrorEnabled = java.lang.Boolean.parseBoolean(value) + (target as TextInputLayout).isErrorEnabled = value.toBoolean() } `@JvmStatic` fun setCounterEnabled(target: View, value: String, context: Context) { - (target as TextInputLayout).isCounterEnabled = java.lang.Boolean.parseBoolean(value) + (target as TextInputLayout).isCounterEnabled = value.toBoolean() }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/callers/text/TextInputLayoutCaller.kt` around lines 24 - 37, Replace the Java boolean parsing with Kotlin idiom in the TextInputLayout caller methods: in setHintEnabled, setErrorEnabled, and setCounterEnabled, stop using java.lang.Boolean.parseBoolean(value) and use the Kotlin String.toBoolean() extension (i.e., convert value.toBoolean()) when assigning to TextInputLayout.isHintEnabled, isErrorEnabled, and isCounterEnabled so the code is idiomatic and null-safe for Kotlin strings.layouteditor/src/main/assets/attributes/attributes.json (1)
642-707: 💤 Low valueConsider removing duplicate attribute definitions.
The
TextInputEditTextattribute mappings (lines 642-707) are identical toEditText(lines 382-447). SinceTextInputEditTextextendsEditTextandTextInputEditTextCallerextendsEditTextCaller, the attribute system might already support inheritance. If so, you could omit this section and rely on inherited mappings, reducing duplication.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@layouteditor/src/main/assets/attributes/attributes.json` around lines 642 - 707, The JSON block for "com.google.android.material.textfield.TextInputEditText" duplicates the same attribute mappings already defined for EditText (e.g., android:hint, android:inputType, android:textColorHint, android:singleLine) and uses text.TextInputEditTextCaller which extends EditTextCaller; remove this redundant block to avoid duplication, or replace it with a single inheritance/delegation entry so the system uses the EditText mappings (refer to text.TextInputEditTextCaller and EditTextCaller and the duplicated attributes like "android:hint" and "android:inputType") to ensure mappings are inherited rather than redefined.layouteditor/src/main/assets/palette/text.json (1)
142-150: ⚡ Quick winIcon name may not match the widget type.
Line 145 uses
"ic_palette_linear_layout_vert"forTextInputLayout. This icon name suggests it's a vertical LinearLayout icon, which doesn't visually represent a text input field. Consider using a more appropriate icon like"ic_palette_edit_text"(as done for TextInputEditText) or creating a dedicated icon for Material text fields.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@layouteditor/src/main/assets/palette/text.json` around lines 142 - 150, The palette entry for the TextInputLayout widget uses an incorrect iconName ("ic_palette_linear_layout_vert") which doesn't represent a text field; update the "iconName" for the entry with name "TextInputLayout" and className "org.appdevforall.codeonthego.layouteditor.editor.palette.text.TextInputLayoutDesign" to a more appropriate icon such as "ic_palette_edit_text" or provide a dedicated Material text field icon, ensuring the new icon asset exists in the project's resources and references the same filename in this JSON entry.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/callers/text/TextInputLayoutCaller.kt`:
- Around line 12-22: The setHint function uses a force-unwrapped project
(project!!.stringsPath) which can throw NPE; modify setHint in
TextInputLayoutCaller to null-check ProjectManager.instance.openedProject (the
local variable project) before using stringsPath and handle the null case
gracefully—either skip resource lookup and keep finalValue as-is or return
early; ensure you reference ProjectManager.instance.openedProject and
ValuesManager.getValueFromResources(ValuesResourceParser.TAG_STRING, finalValue,
project.stringsPath) only after confirming project != null and log or comment
the fallback behavior.
In
`@layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/palette/text/TextInputEditTextDesign.kt`:
- Around line 14-37: The stroke is being drawn twice when isBlueprint is true
because both draw() and dispatchDraw() call Utils.drawDashPathStroke; to fix it,
update dispatchDraw so it only draws the stroke when drawStrokeEnabled is true
AND isBlueprint is false (i.e., change the condition in dispatchDraw to if
(drawStrokeEnabled && !isBlueprint) before calling Utils.drawDashPathStroke),
leaving draw() behavior for blueprint mode unchanged; reference methods/fields:
dispatchDraw, draw, drawStrokeEnabled, isBlueprint, and
Utils.drawDashPathStroke.
---
Outside diff comments:
In
`@layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/views/StructureView.kt`:
- Around line 148-163: The view type detection incorrectly uses
view.javaClass.superclass.simpleName (in the else branch where viewSimpleName is
set), which bypasses mappings for TextInputLayout/TextInputEditText; change the
logic to use view.javaClass.simpleName to look up imgMap (falling back to
imgMap["_unknown"] as before) and update any related display text
(viewName.text) to use that actual simpleName so TextInput* entries resolve
correctly; ensure this change touches the variables viewSimpleName and
imageResource lookup in StructureView (keeping the existing LinearLayout
special-case intact).
---
Duplicate comments:
In
`@layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/palette/text/TextInputLayoutDesign.kt`:
- Around line 14-37: The stroke is being drawn in both dispatchDraw and draw
when isBlueprint and drawStrokeEnabled are true; centralize drawing to avoid
double-draw by removing the stroke call from draw and making dispatchDraw
responsible for all stroke drawing. Change draw(Canvas) to always call
super.draw(canvas) (remove the Utils.drawDashPathStroke call there), and update
dispatchDraw(Canvas) to draw the dashed stroke when (drawStrokeEnabled ||
isBlueprint) using Utils.drawDashPathStroke(this, canvas, if (isBlueprint)
Constants.BLUEPRINT_DASH_COLOR else Constants.DESIGN_DASH_COLOR). Keep
setStrokeEnabled, drawStrokeEnabled and isBlueprint logic unchanged.
---
Nitpick comments:
In `@layouteditor/src/main/assets/attributes/attributes.json`:
- Around line 642-707: The JSON block for
"com.google.android.material.textfield.TextInputEditText" duplicates the same
attribute mappings already defined for EditText (e.g., android:hint,
android:inputType, android:textColorHint, android:singleLine) and uses
text.TextInputEditTextCaller which extends EditTextCaller; remove this redundant
block to avoid duplication, or replace it with a single inheritance/delegation
entry so the system uses the EditText mappings (refer to
text.TextInputEditTextCaller and EditTextCaller and the duplicated attributes
like "android:hint" and "android:inputType") to ensure mappings are inherited
rather than redefined.
In `@layouteditor/src/main/assets/palette/text.json`:
- Around line 142-150: The palette entry for the TextInputLayout widget uses an
incorrect iconName ("ic_palette_linear_layout_vert") which doesn't represent a
text field; update the "iconName" for the entry with name "TextInputLayout" and
className
"org.appdevforall.codeonthego.layouteditor.editor.palette.text.TextInputLayoutDesign"
to a more appropriate icon such as "ic_palette_edit_text" or provide a dedicated
Material text field icon, ensuring the new icon asset exists in the project's
resources and references the same filename in this JSON entry.
In
`@layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/callers/text/TextInputLayoutCaller.kt`:
- Around line 24-37: Replace the Java boolean parsing with Kotlin idiom in the
TextInputLayout caller methods: in setHintEnabled, setErrorEnabled, and
setCounterEnabled, stop using java.lang.Boolean.parseBoolean(value) and use the
Kotlin String.toBoolean() extension (i.e., convert value.toBoolean()) when
assigning to TextInputLayout.isHintEnabled, isErrorEnabled, and isCounterEnabled
so the code is idiomatic and null-safe for Kotlin strings.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 77d6fa7e-a577-46dc-bbce-b0373bfdca12
📒 Files selected for processing (12)
layouteditor/src/main/assets/attributes/attributes.jsonlayouteditor/src/main/assets/palette/text.jsonlayouteditor/src/main/assets/widgetclasses.jsonlayouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/DesignEditor.ktlayouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/callers/TextInputLayoutCaller.javalayouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/callers/text/TextInputEditTextCaller.ktlayouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/callers/text/TextInputLayoutCaller.ktlayouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/palette/text/TextInputEditTextDesign.ktlayouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/palette/text/TextInputLayoutDesign.ktlayouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/tools/XmlLayoutGenerator.javalayouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/tools/XmlLayoutParser.ktlayouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/views/StructureView.kt
💤 Files with no reviewable changes (1)
- layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/callers/TextInputLayoutCaller.java
Jira ticket
Add support for Material
TextInputLayoutandTextInputEditTextso they are previewed correctly in the layout editor