ADFA-3634 Swap logo on tablet/dex left rail on project screen#1205
Merged
hal-eisen-adfa merged 1 commit intostagefrom Apr 18, 2026
Merged
Conversation
Contributor
📝 WalkthroughRelease NotesFeatures & Changes:
Risks & Best Practices Concerns:
WalkthroughThis PR refines the navigation rail menu scrolling logic to robustly locate menu views by scanning child elements, updates the sidebar header branding drawable, and introduces new landscape and tablet layout variants for the editor sidebar UI. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/res/layout/layout_editor_sidebar_header.xml (1)
24-34:⚠️ Potential issue | 🟡 MinorMark the header logo as decorative for accessibility.
If this logo is not actionable and does not convey unique screen content, keep it out of TalkBack traversal.
♿ Proposed fix
<com.google.android.material.imageview.ShapeableImageView android:layout_width="@dimen/sidebar_header_logo_size" android:layout_height="@dimen/sidebar_header_logo_size" android:layout_gravity="center_horizontal" + android:importantForAccessibility="no" android:layout_marginStart="@dimen/sidebar_header_logo_padding"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/res/layout/layout_editor_sidebar_header.xml` around lines 24 - 34, The header logo ShapeableImageView is decorative and should be excluded from accessibility traversal; update the ShapeableImageView element (the one using app:srcCompat="@drawable/cogo_brand_mark") to mark it decorative by adding android:contentDescription="@null" and android:importantForAccessibility="no" so TalkBack ignores it.
🧹 Nitpick comments (1)
app/src/main/java/com/itsaky/androidide/ui/IdeNavigationRailView.kt (1)
6-6: Use Material's public menu accessor instead of scanning child internals.
NavigationBarViewexposesgetMenuViewGroup()in the public API, which avoids coupling this subclass to the direct child order and the concreteNavigationBarMenuViewimplementation. The Material version used (1.12.0) fully supports this API.♻️ Proposed refactor
-import com.google.android.material.navigation.NavigationBarMenuView import com.google.android.material.navigationrail.NavigationRailView @@ - val menuView = (0 until childCount) - .map { getChildAt(it) } - .firstOrNull { it is NavigationBarMenuView } - ?: return@post + val menuView = menuViewGroup if (menuView.parent is NestedScrollView) return@post🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/itsaky/androidide/ui/IdeNavigationRailView.kt` at line 6, The subclass IdeNavigationRailView currently imports and relies on the concrete NavigationBarMenuView and child ordering; replace that internal scanning with the public API NavigationBarView.getMenuViewGroup(): call getMenuViewGroup() where you currently locate the menu view (e.g., in the constructor or initialization code of IdeNavigationRailView), treat the result as a ViewGroup instead of NavigationBarMenuView, remove the direct import of com.google.android.material.navigation.NavigationBarMenuView, and update any casts/usages to use the returned ViewGroup or safe APIs on it; this avoids coupling to internal child ordering and the concrete implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/res/layout-land/fragment_editor_sidebar.xml`:
- Around line 18-23: Change the resource directory qualifier for the
fragment_editor_sidebar layout so it only applies to tablets in landscape:
rename/move the current layout file that lives under the "layout-land" qualifier
to the "layout-sw600dp-land" qualifier so the ConstraintLayout (the root element
in fragment_editor_sidebar.xml) and its header reference
(`@layout/layout_editor_sidebar_header`) are only used on devices with sw>=600dp
in landscape; ensure the filename stays fragment_editor_sidebar.xml and update
any references if your build system requires it.
---
Outside diff comments:
In `@app/src/main/res/layout/layout_editor_sidebar_header.xml`:
- Around line 24-34: The header logo ShapeableImageView is decorative and should
be excluded from accessibility traversal; update the ShapeableImageView element
(the one using app:srcCompat="@drawable/cogo_brand_mark") to mark it decorative
by adding android:contentDescription="@null" and
android:importantForAccessibility="no" so TalkBack ignores it.
---
Nitpick comments:
In `@app/src/main/java/com/itsaky/androidide/ui/IdeNavigationRailView.kt`:
- Line 6: The subclass IdeNavigationRailView currently imports and relies on the
concrete NavigationBarMenuView and child ordering; replace that internal
scanning with the public API NavigationBarView.getMenuViewGroup(): call
getMenuViewGroup() where you currently locate the menu view (e.g., in the
constructor or initialization code of IdeNavigationRailView), treat the result
as a ViewGroup instead of NavigationBarMenuView, remove the direct import of
com.google.android.material.navigation.NavigationBarMenuView, and update any
casts/usages to use the returned ViewGroup or safe APIs on it; this avoids
coupling to internal child ordering and the concrete implementation.
🪄 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: 1b3f5b61-6e16-4c3a-9940-106795013fc7
📒 Files selected for processing (4)
app/src/main/java/com/itsaky/androidide/ui/IdeNavigationRailView.ktapp/src/main/res/layout-land/fragment_editor_sidebar.xmlapp/src/main/res/layout-sw600dp/fragment_editor_sidebar.xmlapp/src/main/res/layout/layout_editor_sidebar_header.xml
jomen-adfa
approved these changes
Apr 18, 2026
hal-eisen-adfa
deleted the
ADFA-3634-Update-icon-in-upper-left-of-landscape-tablet-Dex
branch
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
2 participants
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.
This one was slightly more tricky. Updated enableMenuScrolling() to find the menu by type, then the headerLayout approach works safely.