ADFA-2474 Shrink feedback FAB from 56dp to 32dp - #780
Conversation
hal-eisen-adfa
changed the title
|
Tip For best results, initiate chat on the files or code changes. @hal-eisen-adfa: Hello! How can I assist you with this PR? I can help with:
Just let me know what you'd like me to do! 🧠 Learnings used |
📝 WalkthroughRelease Notes: Feedback FAB Size ReductionChanges
Risks & Considerations
WalkthroughAdds a custom size attribute ( Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
Pre-merge checks✅ Passed checks (3 passed)
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[bot]](https://cdn.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Fix all issues with AI Agents 🤖
In @app/src/main/res/layout/activity_about.xml:
- Around line 123-124: The layouts reference a missing drawable name
baseline_feedback_64 which will break builds; either add a vector drawable
resource named baseline_feedback_64 (create a 24–32dp SVG-based/vector drawable
matching the FAB size and place it in res/drawable as baseline_feedback_64.xml)
or update every layout that uses android:src="@drawable/baseline_feedback_64"
(e.g., activity_about.xml, activity_main.xml, activity_editor.xml,
activity_preferences.xml, activity_contributors.xml,
activity_plugin_manager.xml, termux-app, layout-editor) to use an existing icon
such as @drawable/ic_send and ensure the fabCustomSize (32dp) and any tint
attributes remain appropriate for the chosen drawable.
In @app/src/main/res/layout/activity_contributors.xml:
- Around line 127-128: The FAB is using a 64dp asset scaled down to 32dp which
causes visual degradation; replace the
android:src="@drawable/baseline_feedback_64" with a 32dp or 24dp variant (e.g.,
baseline_feedback_32 or baseline_feedback_24) or swap in the Material vector
drawable at the correct size, keeping app:fabCustomSize="32dp" as-is so the
touch target remains accessible.
In @termux/termux-app/src/main/res/layout/activity_termux.xml:
- Around line 109-110: The FAB uses app:fabCustomSize="32dp" with
android:src="@drawable/baseline_feedback_64", which produces a touch target
below the Material Design 48×48 dp minimum; fix by either (A) increasing
app:fabCustomSize to at least 48dp, (B) keep the 32dp visual size but expand the
touch area to 48dp using a TouchDelegate or wrapping the FAB with a larger
container view that forwards touches, or (C) also replace the icon with
@drawable/baseline_feedback_24 for semantic correctness; ensure after changes
the effective touch area meets 48×48 dp accessibility requirements before
merging.
♻️ Duplicate comments (4)
app/src/main/res/layout/activity_editor.xml (1)
78-78: Verify icon scaling with the reduced FAB size.Same concern as in other layouts: the 64dp icon resource may not scale optimally in the 32dp FAB container.
layouteditor/src/main/res/layout/activity_layout_editor.xml (1)
179-179: Verify icon scaling with the reduced FAB size.Same concern as in other layouts: the 64dp icon resource may not scale optimally in the 32dp FAB container.
app/src/main/res/layout/activity_main.xml (1)
86-86: Verify icon scaling with the reduced FAB size.Same concern as in other layouts: the 64dp icon resource may not scale optimally in the 32dp FAB container.
app/src/main/res/layout/activity_preferences.xml (1)
59-60: Verify icon scaling with the reduced FAB size.Same concern as in other layouts: the 64dp icon resource may not scale optimally in the 32dp FAB container.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
app/src/main/res/layout/activity_about.xmlapp/src/main/res/layout/activity_contributors.xmlapp/src/main/res/layout/activity_editor.xmlapp/src/main/res/layout/activity_main.xmlapp/src/main/res/layout/activity_plugin_manager.xmlapp/src/main/res/layout/activity_preferences.xmllayouteditor/src/main/res/layout/activity_layout_editor.xmltermux/termux-app/src/main/res/layout/activity_termux.xml
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Daniel-ADFA
Repo: appdevforall/CodeOnTheGo PR: 678
File: cv-image-to-xml/src/main/res/layout/activity_computer_vision.xml:68-78
Timestamp: 2025-12-30T20:49:31.269Z
Learning: In the CodeOnTheGo project, FloatingActionButton positions in layouts are managed programmatically by the feedbackButtonManager class, so ConstraintLayout runtime constraints are not required in the XML layout files for FABs.
📚 Learning: 2025-12-30T20:49:31.269Z
Learnt from: Daniel-ADFA
Repo: appdevforall/CodeOnTheGo PR: 678
File: cv-image-to-xml/src/main/res/layout/activity_computer_vision.xml:68-78
Timestamp: 2025-12-30T20:49:31.269Z
Learning: In the CodeOnTheGo project, FloatingActionButton positions in layouts are managed programmatically by the feedbackButtonManager class, so ConstraintLayout runtime constraints are not required in the XML layout files for FABs.
Applied to files:
app/src/main/res/layout/activity_contributors.xmlapp/src/main/res/layout/activity_preferences.xmlapp/src/main/res/layout/activity_about.xmllayouteditor/src/main/res/layout/activity_layout_editor.xmlapp/src/main/res/layout/activity_plugin_manager.xmlapp/src/main/res/layout/activity_main.xmltermux/termux-app/src/main/res/layout/activity_termux.xmlapp/src/main/res/layout/activity_editor.xml
🔇 Additional comments (2)
termux/termux-app/src/main/res/layout/activity_termux.xml (1)
2-2: LGTM!The app namespace declaration is correctly added to support the
app:fabCustomSizeattribute used in the FloatingActionButton below.app/src/main/res/layout/activity_plugin_manager.xml (1)
87-88: AddensureMinTouchTargetSize="true"to meet accessibility guidelines for the 32dp FAB.The 32dp visual size falls below the recommended 48dp minimum touch target for accessibility. However, Android's FloatingActionButton provides
ensureMinTouchTargetSizeattribute to automatically expand the touch bounds to 48dp while maintaining the smaller visual size.The downscaling concern is not applicable here—
baseline_feedback_64andbaseline_feedback_24are vector drawables (resolution-independent), so scaling has no visual quality impact.Recommended fix: Add
app:ensureMinTouchTargetSize="true"to the FAB definition to ensure compliance without requiring icon variants.Likely an incorrect or invalid review comment.
Shrink feedback FAB from 56dp to 32dp
1 participant
Many people find the large FAB distracting and inconvenient. Trying a smaller approach.