fix(ADFA-2872): Set default layout params values#1158
Conversation
📝 WalkthroughRelease NotesBug Fix: NullPointerException During Layout Preview (ADFA-2872)Changes:
|
| Cohort / File(s) | Summary |
|---|---|
Null Guards for Layout Params layouteditor/.../callers/ViewCaller.java |
Added null checks in setLayoutWidth and setLayoutHeight methods before accessing getLayoutParams() to prevent potential NullPointerException. |
Default Layout Params Assignment layouteditor/.../tools/XMLParserUtils.kt, layouteditor/.../tools/XmlLayoutParser.kt |
Both XML parsing utilities now explicitly assign ViewGroup.LayoutParams with WRAP_CONTENT dimensions to newly created placeholder views and generic views during parsing. |
Estimated code review effort
🎯 2 (Simple) | ⏱️ ~10 minutes
Suggested reviewers
- jatezzz
- Daniel-ADFA
- jomen-adfa
- itsaky-adfa
Poem
🐰 Hop, hop, the layouts now are sound,
With null checks and defaults around,
No more crashes in the night,
Every param wrapped up tight! 📐✨
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title accurately describes the main change: setting default layout params values to fix a NullPointerException issue, which aligns with all three file changes. |
| Description check | ✅ Passed | The description is concise and directly related to the changeset, explaining the fix addresses NullPointerException when previewing layouts. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
bug/ADFA-2872-layout-editor-npe
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 @coderabbitai help to get the list of available commands and usage tips.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/callers/ViewCaller.java (1)
19-31: Null guards fix the NPE, but silent failure may obscure issues.The null checks correctly prevent the crash. However, silently returning when
layoutParamsis null could make debugging harder if a layout attribute is unexpectedly not applied.Consider adding a log statement when skipping:
📝 Optional: Add logging for observability
public static void setLayoutWidth(View target, String value, Context context) { - if (target.getLayoutParams() != null) { + if (target.getLayoutParams() == null) { + android.util.Log.w("ViewCaller", "Skipping setLayoutWidth: layoutParams is null"); + return; + } - target.getLayoutParams().width = (int) DimensionUtil.parse(value, context); - target.requestLayout(); - } + target.getLayoutParams().width = (int) DimensionUtil.parse(value, context); + target.requestLayout(); } public static void setLayoutHeight(View target, String value, Context context) { - if (target.getLayoutParams() != null) { + if (target.getLayoutParams() == null) { + android.util.Log.w("ViewCaller", "Skipping setLayoutHeight: layoutParams is null"); + return; + } - target.getLayoutParams().height = (int) DimensionUtil.parse(value, context); - target.requestLayout(); - } + target.getLayoutParams().height = (int) DimensionUtil.parse(value, context); + target.requestLayout(); }Also note:
RelativeLayoutCallerandFrameLayoutCallerhave similar methods accessinggetLayoutParams()without null checks (per relevant code snippets). If those code paths can be reached with views lacking layout params, they remain vulnerable to NPE.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/callers/ViewCaller.java` around lines 19 - 31, setLayoutWidth and setLayoutHeight currently guard against NPE by returning when target.getLayoutParams() is null, but do so silently; update both methods to log a warning when layoutParams is null (include target.toString() or identifying info and the incoming value) so skipped updates are observable, and apply the same null-check + warning pattern to the similar methods in RelativeLayoutCaller and FrameLayoutCaller that call getLayoutParams() to prevent hidden failures while still avoiding crashes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/callers/ViewCaller.java`:
- Around line 19-31: setLayoutWidth and setLayoutHeight currently guard against
NPE by returning when target.getLayoutParams() is null, but do so silently;
update both methods to log a warning when layoutParams is null (include
target.toString() or identifying info and the incoming value) so skipped updates
are observable, and apply the same null-check + warning pattern to the similar
methods in RelativeLayoutCaller and FrameLayoutCaller that call
getLayoutParams() to prevent hidden failures while still avoiding crashes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 65c4cc88-01ad-4e6e-874d-74a475df3d94
📒 Files selected for processing (3)
layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/callers/ViewCaller.javalayouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/tools/XMLParserUtils.ktlayouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/tools/XmlLayoutParser.kt
Fix
NullPointerExceptionwhen previewing a layout