ADFA-2686: Centralize the feedback FAB into one consistent size#1507
Open
hal-eisen-adfa wants to merge 3 commits into
Open
ADFA-2686: Centralize the feedback FAB into one consistent size#1507hal-eisen-adfa wants to merge 3 commits into
hal-eisen-adfa wants to merge 3 commits into
Conversation
The "Send feedback" FAB was declared independently at every call site, so its size drifted: OnboardingActivity built it in code with no fabCustomSize (defaulting to Material's 56dp), while the five XML screens used 32dp. Define the FAB's appearance once in res/layout/feedback_fab.xml (size, icon, elevation, content description) and reuse it via <include> in the five activity layouts and via LayoutInflater in OnboardingActivity. Size is driven by new feedback_fab_size (32dp) and feedback_fab_icon_size dimens, the latter controlling how snugly the icon fills the circle. Since the FAB now comes from an included binding, call sites reference binding.fabFeedback.root (nullable ?.root in AboutActivity, whose land/ sw600dp variants omit the FAB). Drag/position/click behavior in FeedbackButtonManager is unchanged. Verified on emulator: the feedback FAB now renders as the same 32dp circle on the editor, main, about, preferences, plugin-manager and onboarding screens (onboarding was previously the 56dp outlier).
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
Contributor
|
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 (10)
🚧 Files skipped from review as they are similar to previous changes (9)
📝 Walkthrough
WalkthroughThe feedback FAB is centralized in a shared layout with reusable dimensions. Activity layouts include it, onboarding inflates it directly, and feedback manager initialization now receives the included FAB root view. ChangesFeedback FAB integration
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/main/res/layout/feedback_fab.xml (1)
15-16: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winPrefer
app:elevationoverandroid:elevationfor FloatingActionButton.
FloatingActionButtonmanages its own shadow/elevation internally via the Material component library. Usingandroid:elevationcan interfere with the FAB's shadow rendering on certain API levels.app:elevationis the Material Components-recommended attribute. Additionally, considerapp:srcCompatinstead ofandroid:srcfor consistency with other FABs in the codebase (e.g.,fab_install_plugininactivity_plugin_manager.xmlusesapp:srcCompat).♻️ Proposed refactor
- android:elevation="16dp" - android:src="`@drawable/baseline_feedback_64`" + app:elevation="16dp" + app:srcCompat="`@drawable/baseline_feedback_64`"🤖 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 `@app/src/main/res/layout/feedback_fab.xml` around lines 15 - 16, Update the FloatingActionButton attributes in feedback_fab.xml to use app:elevation instead of android:elevation, and replace android:src with app:srcCompat to match the established FAB pattern used by fab_install_plugin.
🤖 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.
Nitpick comments:
In `@app/src/main/res/layout/feedback_fab.xml`:
- Around line 15-16: Update the FloatingActionButton attributes in
feedback_fab.xml to use app:elevation instead of android:elevation, and replace
android:src with app:srcCompat to match the established FAB pattern used by
fab_install_plugin.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b7f70916-2d28-4365-a866-5e1ff4aff02c
📒 Files selected for processing (13)
app/src/main/java/com/itsaky/androidide/activities/AboutActivity.ktapp/src/main/java/com/itsaky/androidide/activities/MainActivity.ktapp/src/main/java/com/itsaky/androidide/activities/OnboardingActivity.ktapp/src/main/java/com/itsaky/androidide/activities/PluginManagerActivity.ktapp/src/main/java/com/itsaky/androidide/activities/PreferencesActivity.ktapp/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.ktapp/src/main/res/layout/activity_about.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.xmlapp/src/main/res/layout/feedback_fab.xmlresources/src/main/res/values/dimens.xml
The land and sw600dp variants of activity_about.xml omitted the feedback FAB, which violates the invariant that every page must always offer feedback. Add the shared feedback_fab include to both variants (bottom|start, matching portrait). Now that fab_feedback is present in every activity_about variant, its View Binding field is non-null, so AboutActivity uses binding.fabFeedback.root (dropping the previous nullable ?.root) — consistent with the other screens.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/main/res/layout-land/activity_about.xml (1)
121-128: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a shared dimension resource for the feedback FAB margin instead of hardcoded
16dp. Both About variants hardcode16dpon the FAB include, while the rest of these files consistently use@dimen/resources for margins. This breaks the PR's centralization goal.
app/src/main/res/layout-land/activity_about.xml#L121-L128: replaceandroid:layout_margin="16dp"with a shared dimension reference (e.g.,@dimen/feedback_fab_margin).app/src/main/res/layout-sw600dp/activity_about.xml#L121-L128: apply the same dimension reference.🤖 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 `@app/src/main/res/layout-land/activity_about.xml` around lines 121 - 128, Replace the hardcoded feedback FAB margin in the include blocks of app/src/main/res/layout-land/activity_about.xml lines 121-128 and app/src/main/res/layout-sw600dp/activity_about.xml lines 121-128 with the shared feedback FAB dimension resource, such as `@dimen/feedback_fab_margin`.
🤖 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.
Nitpick comments:
In `@app/src/main/res/layout-land/activity_about.xml`:
- Around line 121-128: Replace the hardcoded feedback FAB margin in the include
blocks of app/src/main/res/layout-land/activity_about.xml lines 121-128 and
app/src/main/res/layout-sw600dp/activity_about.xml lines 121-128 with the shared
feedback FAB dimension resource, such as `@dimen/feedback_fab_margin`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e802edc4-c2d0-4815-ad9b-28cc6309093d
📒 Files selected for processing (3)
app/src/main/java/com/itsaky/androidide/activities/AboutActivity.ktapp/src/main/res/layout-land/activity_about.xmlapp/src/main/res/layout-sw600dp/activity_about.xml
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/com/itsaky/androidide/activities/AboutActivity.kt
- feedback_fab.xml: use app:elevation and app:srcCompat (Material-recommended attributes) instead of android:elevation / android:src. - Centralize the FAB margin: add @dimen/feedback_fab_margin (16dp) and reference it from all 7 feedback-FAB includes (editor/main/about/preferences/plugin_manager plus about land/sw600dp variants) instead of hardcoding 16dp. - OnboardingActivity: read the same dimen via getDimensionPixelSize instead of a hardcoded 16dp TypedValue conversion, so the margin lives in one place.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The "Send feedback" FAB was declared independently at every call site, so its size drifted.
OnboardingActivitybuilt it in code with nofabCustomSize(defaulting to Material's 56dp), while the five XML screens used 32dp. Per the ticket, this centralizes the FAB into one definition so it's the same (smaller) size everywhere.Fixes ADFA-2686 — Feedback button wrong size.
Changes
app/src/main/res/layout/feedback_fab.xml— single source of the FAB's appearance (size, icon, elevation, content description).resources/.../dimens.xml:feedback_fab_size(32dp) andfeedback_fab_icon_size(controls how snugly the icon fills the circle).activity_editor/main/about/preferences/plugin_manager.xml) now<include>the shared FAB, supplying only positioning (uniform bottom|start + 16dp margin).OnboardingActivityinflates the shared layout instead of hand-building the FAB — fixing the 56dp outlier.binding.fabFeedback.root(nullable?.rootinAboutActivity, whoseland/sw600dpvariants omit the FAB) because the FAB now comes from an included binding.FeedbackButtonManagerbehavior (drag / position persistence / click → feedback email flow / long-press tooltip) is unchanged.Verification
Built
:app:AssembleV8Debug, installed on the emulator, and confirmed the feedback FAB renders as the same 32dp circle (48dp Material touch target) on the editor, main, about, preferences, plugin-manager, and onboarding screens. Onboarding was the previous 56dp outlier — verified by clearing app data to force the real onboarding flow and comparing measured bounds (144px view now vs. 168px/56dp before).fabCustomSize)