ADFA-3123 | Adaptive navigation patterns#1055
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRemoves orientation-driven toolbar rearrangement and DiffUtil list diffing; replaces Flexbox-based template grid with a GridLayoutManager that recalculates span counts on configuration changes; refactors the editor AppBar to a FlexboxLayout containing a ProjectActionsToolbar and adjusts related layout widths and item sizing. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 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: 3
🤖 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/java/com/itsaky/androidide/fragments/TemplateListFragment.kt`:
- Around line 79-84: The tooltip is being anchored to binding.root inside the
setOnLongClickListener attached to binding.exitButton, which can position it far
from the pressed control; update the call to
TooltipManager.showIdeCategoryTooltip (inside the
binding.exitButton.setOnLongClickListener block) to use the exit button as the
anchor (e.g., anchorView = binding.exitButton or the listener's view parameter)
while keeping context=requireContext() and tag=EXIT_TO_MAIN so the tooltip
appears next to the long-pressed button.
- Around line 68-73: The span calculation currently uses
resources.configuration.screenWidthDp; change to compute span count from the
actual RecyclerView width by adding a helper method (e.g.,
updateSpanCount(minItemWidthDp: Int)) in TemplateListFragment that measures
binding.list.width (after layout) and sets gridLayoutManager.spanCount
accordingly, call this helper from binding.list.doOnLayout for initial setup and
from onConfigurationChanged() to recompute on rotations/size changes; ensure you
keep the GridLayoutManager instance (gridLayoutManager) and update its spanCount
rather than recreating the adapter or list.
In `@app/src/main/res/layout/content_editor.xml`:
- Around line 65-74: The ProjectActionsToolbar is declared wrap_content but
inflates project_actions_toolbar.xml whose root MaterialToolbar uses
android:layout_width="match_parent", causing measurement ambiguity; open
project_actions_toolbar.xml and change the root MaterialToolbar's
android:layout_width to "wrap_content" (and likewise change any immediate child
views that use match_parent to wrap_content or use wrap_content with appropriate
paddings) so the custom view ProjectActionsToolbar can size itself correctly
within the FlexboxLayout.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5c2c5a9c-d4dd-4eef-8f67-57efbc14f805
📒 Files selected for processing (5)
app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.ktapp/src/main/java/com/itsaky/androidide/adapters/TemplateListAdapter.ktapp/src/main/java/com/itsaky/androidide/fragments/TemplateListFragment.ktapp/src/main/res/layout/content_editor.xmlapp/src/main/res/layout/layout_template_list_item.xml
💤 Files with no reviewable changes (1)
- app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt
app/src/main/java/com/itsaky/androidide/fragments/TemplateListFragment.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/itsaky/androidide/fragments/TemplateListFragment.kt
Show resolved
Hide resolved
b66cbf0 to
290fe77
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/main/java/com/itsaky/androidide/fragments/TemplateListFragment.kt (1)
99-111: Consider using actual RecyclerView width for span calculation.The current approach uses orientation and item count, but doesn't account for the actual available width of
binding.list. This can cause issues in constrained layouts (split-screen, foldables, embedded containers) where the RecyclerView may be narrower than expected.Using
binding.list.widthafter layout would provide more accurate span counts across all scenarios.Suggested approach using actual width
+import androidx.core.view.doOnLayout ... + private val minItemWidthDp = 160 + override fun onConfigurationChanged(newConfig: Configuration) { super.onConfigurationChanged(newConfig) - - updateSpanCount() + _binding?.list?.post { updateSpanCount() } } private fun updateSpanCount() { - val isLandscape = resources.configuration.orientation == Configuration.ORIENTATION_LANDSCAPE - val maxSpans = if (isLandscape) 6 else 4 - + val listWidthPx = binding.list.width + if (listWidthPx <= 0) return + + val minItemWidthPx = (minItemWidthDp * resources.displayMetrics.density).toInt() val itemCount = binding.list.adapter?.itemCount ?: 0 - - val optimalSpans = maxOf(1, minOf(maxSpans, itemCount)) - + val widthBasedSpans = (listWidthPx / minItemWidthPx).coerceIn(1, 6) + val optimalSpans = minOf(widthBasedSpans, maxOf(1, itemCount)) + val layoutManager = binding.list.layoutManager as? GridLayoutManager if (layoutManager != null && layoutManager.spanCount != optimalSpans) { layoutManager.spanCount = optimalSpans } }Also update the initial setup to use
doOnLayout:binding.list.layoutManager = gridLayoutManager binding.list.doOnLayout { updateSpanCount() }🤖 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/fragments/TemplateListFragment.kt` around lines 99 - 111, The updateSpanCount currently bases spans on orientation and item count only; change it to measure the actual available width of binding.list (use binding.list.width after layout) and compute optimalSpans from availableWidth divided by a target minimum column width (e.g., desiredItemMinWidth), clamped between 1 and maxSpans; update the GridLayoutManager.spanCount accordingly in updateSpanCount; also ensure updateSpanCount is invoked after layout by calling binding.list.doOnLayout { updateSpanCount() } (and/or attach an OnLayoutChangeListener) where the layout manager is set so width measurement is valid.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/src/main/java/com/itsaky/androidide/fragments/TemplateListFragment.kt`:
- Around line 99-111: The updateSpanCount currently bases spans on orientation
and item count only; change it to measure the actual available width of
binding.list (use binding.list.width after layout) and compute optimalSpans from
availableWidth divided by a target minimum column width (e.g.,
desiredItemMinWidth), clamped between 1 and maxSpans; update the
GridLayoutManager.spanCount accordingly in updateSpanCount; also ensure
updateSpanCount is invoked after layout by calling binding.list.doOnLayout {
updateSpanCount() } (and/or attach an OnLayoutChangeListener) where the layout
manager is set so width measurement is valid.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 46fe4dd9-d078-4c5b-9a0b-dd6e86b9cacd
📒 Files selected for processing (6)
app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.ktapp/src/main/java/com/itsaky/androidide/adapters/TemplateListAdapter.ktapp/src/main/java/com/itsaky/androidide/fragments/TemplateListFragment.ktapp/src/main/res/layout/content_editor.xmlapp/src/main/res/layout/layout_template_list_item.xmlcommon/src/main/res/layout/project_actions_toolbar.xml
💤 Files with no reviewable changes (1)
- app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/res/layout/layout_template_list_item.xml
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/main/res/layout-land/content_editor.xml (1)
41-47: Remove the unsupportedlayout_weightattribute from the Flexbox child.
FlexboxLayoutdoes not honorandroid:layout_weight—that attribute is forLinearLayoutonly. This row usesFlexboxLayoutand already declaresapp:layout_flexGrow="1"for expansion. Remove the redundantandroid:layout_weighton line 47.♻️ Suggested cleanup
<com.google.android.material.appbar.MaterialToolbar android:id="@+id/title_toolbar" android:layout_width="wrap_content" android:layout_height="wrap_content" app:layout_flexGrow="1" app:layout_flexShrink="0" - android:layout_weight="1" android:minHeight="0dp" android:padding="0dp"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/res/layout-land/content_editor.xml` around lines 41 - 47, Remove the unsupported android:layout_weight attribute from the MaterialToolbar element (id "@+id/title_toolbar") because FlexboxLayout uses app:layout_flexGrow/app:layout_flexShrink for sizing; delete the android:layout_weight="1" attribute so the toolbar relies on app:layout_flexGrow="1" for expansion and avoid redundant/ignored attributes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/src/main/res/layout-land/content_editor.xml`:
- Around line 41-47: Remove the unsupported android:layout_weight attribute from
the MaterialToolbar element (id "@+id/title_toolbar") because FlexboxLayout uses
app:layout_flexGrow/app:layout_flexShrink for sizing; delete the
android:layout_weight="1" attribute so the toolbar relies on
app:layout_flexGrow="1" for expansion and avoid redundant/ignored attributes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 662f729e-a10f-45e8-bfb0-db44154c60c0
📒 Files selected for processing (2)
app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.ktapp/src/main/res/layout-land/content_editor.xml
|
Fixed for DeX mode document_5012592404868368195.mp4 |
app/src/main/java/com/itsaky/androidide/fragments/TemplateListFragment.kt
Show resolved
Hide resolved
Change toolbar to wrap_content and cap grid columns by orientation and item count.
0730f9d to
2f409c0
Compare
This PR refactors the UI to properly support adaptive navigation patterns across different screen sizes and orientations, specifically addressing the editor's toolbars and the project template list.
Changes include:
BaseEditorActivitywith a nativeFlexboxLayoutincontent_editor.xml. This automatically handles wrapping theProjectActionsToolbarbelow thetitle_toolbaron smaller screens and places them side-by-side on larger screens (like tablets or in landscape mode), eliminating the need for complex layout calculations in code.FlexboxLayoutManagerwith a customGlobalLayoutListenerto a dynamicGridLayoutManagerinTemplateListFragment. This natively adjusts the number of columns (between 1 and 4) based on the available screen width, providing a much smoother and more performant reflow when resizing the window (e.g., in DeX mode) or rotating the device.layout_template_list_item.xmlto usematch_parentwidths, ensuring the cards properly fill their assigned grid columns without leaving awkward empty spaces.TemplateListAdapter.Details
1000097917.mp4
Ticket
ADFA-3123
Observation
The use of
GridLayoutManagercombined withonConfigurationChangedfor the template list provides a significant performance improvement over the previousGlobalLayoutListenerapproach, especially in desktop-like environments (Samsung DeX) where window resizing triggers frequent layout passes.