ADFA-3071 | Refactor BottomSheet behavior and clean up attribute dialogs#1179
Merged
Conversation
Contributor
📝 WalkthroughRelease Notes - ADFA-3071: BottomSheet Behavior Refactor & Attribute Dialogs CleanupFeatures & Improvements
|
| Cohort / File(s) | Summary |
|---|---|
Design editor UI & attribute handling layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/DesignEditor.kt |
Rebuilt display key/value generation, replaced inline adapter/binding with AppliedAttributesAdapter / AvailableAttributesAdapter, unified tooltip call sites, switched to android.text.TextWatcher/Editable for search filtering, and configured BottomSheetBehavior properties (isFitToContents, state = STATE_EXPANDED, skipCollapsed). |
Deletion confirmation extraction layouteditor/src/main/java/.../DesignEditor.kt (same file, new helper) |
Extracted inline delete-confirm flow into confirmViewDeletion(target, dialog: BottomSheetDialog) which performs removeId, removeViewAttributes, removeWidget, updateStructure, updateUndoRedoHistory and dismisses the dialog. |
Sequence Diagram(s)
sequenceDiagram
rect rgba(70,130,180,0.5)
participant User
end
rect rgba(46,204,113,0.5)
participant DesignEditor
end
rect rgba(155,89,182,0.5)
participant BottomSheetDialog
end
rect rgba(241,196,15,0.5)
participant Adapter
end
User->>DesignEditor: open attributes (defined/available)
DesignEditor->>BottomSheetDialog: create & configure behavior (fitToContents, expand, skipCollapsed)
DesignEditor->>Adapter: instantiate with click handlers
Adapter->>DesignEditor: on attribute click -> showAttributeEdit / confirm deletion
DesignEditor->>BottomSheetDialog: dismiss (after edit or via confirmViewDeletion)
DesignEditor->>DesignEditor: removeWidget/removeViewAttributes/updateUndoRedoHistory (on deletion)
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested reviewers
- jomen-adfa
- Daniel-ADFA
- itsaky-adfa
Poem
🐇 I hopped into the editor bright,
Adapters tuned and sheets take flight,
I nudged a dialog, tidy and neat,
Removed a view with nimble feet—
Code carrots crisp, the layout's light.
🚥 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 accurately summarizes the main change: refactoring BottomSheet behavior and cleaning up attribute dialogs in the pull request. |
| Description check | ✅ Passed | The description is directly related to the changeset, explaining the rationale for BottomSheetDialog refactoring and providing visual evidence with screenshots. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
refactor/ADFA-3071-bottom-sheet-behavior
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.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/DesignEditor.kt (2)
685-691: Decouple key/value assembly from positional indexing.Line 690 depends on index alignment between
keySet()andvalues(). It works today but is fragile and less maintainable than key-based access.♻️ Suggested refactor
- allKeysAndValues.keySet().forEachIndexed { i, key -> + allKeysAndValues.keySet().forEach { key -> val foundAttrDef = allAttrs.find { it[Constants.KEY_ATTRIBUTE_NAME].toString() == key } if (foundAttrDef != null) { displayKeys.add(key) - displayValues.add(allKeysAndValues.values()[i]) + displayValues.add(allKeysAndValues.getValue(key)) displayAttrs.add(foundAttrDef) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/DesignEditor.kt` around lines 685 - 691, The loop currently relies on positional indexing between allKeysAndValues.keySet() and allKeysAndValues.values(), which is fragile; change the iteration to use key-based access: iterate over entries or keys and retrieve the value by key (e.g., allKeysAndValues[key]) so that when foundAttrDef (found via allAttrs.find { it[Constants.KEY_ATTRIBUTE_NAME].toString() == key }) you push the correct value into displayValues alongside displayKeys and displayAttrs; update the block that references allKeysAndValues.values()[i] to use key-based lookup and keep adding to displayKeys, displayValues, and displayAttrs the corresponding matched items.
772-779: Replace verboseTextWatcherwithdoAfterTextChangedfrom core-ktx.Lines 774–775 are empty overrides that can be eliminated. The
doAfterTextChangedextension function fromandroidx.core.widgetis already used elsewhere in the codebase and is the idiomatic Kotlin approach.♻️ Suggested refactor
-import android.text.Editable -import android.text.TextWatcher +import androidx.core.widget.doAfterTextChanged ... - binding.searchEditText.addTextChangedListener( - object : TextWatcher { - override fun beforeTextChanged(s: CharSequence?, start: Int, count: Int, after: Int) {} - override fun onTextChanged(s: CharSequence?, start: Int, before: Int, count: Int) {} - override fun afterTextChanged(s: Editable?) { - adapter.filter(s?.toString() ?: "") - } - } - ) + binding.searchEditText.doAfterTextChanged { s -> + adapter.filter(s?.toString().orEmpty()) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/DesignEditor.kt` around lines 772 - 779, Replace the anonymous TextWatcher on binding.searchEditText with the core-ktx doAfterTextChanged extension: remove the redundant beforeTextChanged/onTextChanged overrides and call binding.searchEditText.doAfterTextChanged { adapter.filter(it?.toString() ?: "") } instead, keeping the adapter.filter call intact so behavior is unchanged; locate the TextWatcher registration around binding.searchEditText.addTextChangedListener to make this swap.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/DesignEditor.kt`:
- Around line 685-691: The loop currently relies on positional indexing between
allKeysAndValues.keySet() and allKeysAndValues.values(), which is fragile;
change the iteration to use key-based access: iterate over entries or keys and
retrieve the value by key (e.g., allKeysAndValues[key]) so that when
foundAttrDef (found via allAttrs.find {
it[Constants.KEY_ATTRIBUTE_NAME].toString() == key }) you push the correct value
into displayValues alongside displayKeys and displayAttrs; update the block that
references allKeysAndValues.values()[i] to use key-based lookup and keep adding
to displayKeys, displayValues, and displayAttrs the corresponding matched items.
- Around line 772-779: Replace the anonymous TextWatcher on
binding.searchEditText with the core-ktx doAfterTextChanged extension: remove
the redundant beforeTextChanged/onTextChanged overrides and call
binding.searchEditText.doAfterTextChanged { adapter.filter(it?.toString() ?: "")
} instead, keeping the adapter.filter call intact so behavior is unchanged;
locate the TextWatcher registration around
binding.searchEditText.addTextChangedListener to make this swap.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 07554516-bce1-42bb-adb0-05bd6fb54622
📒 Files selected for processing (1)
layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/DesignEditor.kt
jatezzz
requested review from
a team,
Daniel-ADFA,
dara-abijo-adfa and
jomen-adfa
jomen-adfa
approved these changes
Apr 14, 2026
jatezzz
force-pushed
the
refactor/ADFA-3071-bottom-sheet-behavior
branch
from
4488fa4 to
069f193
Compare
Apply fit-to-contents behavior and idiomatic Kotlin refactoring to DesignEditor
jatezzz
force-pushed
the
refactor/ADFA-3071-bottom-sheet-behavior
branch
from
069f193 to
8cfcc41
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/DesignEditor.kt (1)
772-780: Consider usingdoAfterTextChangedextension to simplify TextWatcher.The empty
beforeTextChangedandonTextChangedmethods are boilerplate required by the interface. AndroidX Core KTX providesdoAfterTextChangedwhich is more concise.♻️ Optional: Use doAfterTextChanged extension
Add the import:
import androidx.core.widget.doAfterTextChangedThen simplify the listener:
-binding.searchEditText.addTextChangedListener( - object : TextWatcher { - override fun beforeTextChanged(s: CharSequence?, start: Int, count: Int, after: Int) {} - override fun onTextChanged(s: CharSequence?, start: Int, before: Int, count: Int) {} - override fun afterTextChanged(s: Editable?) { - adapter.filter(s?.toString() ?: "") - } - } -) +binding.searchEditText.doAfterTextChanged { s -> + adapter.filter(s?.toString() ?: "") +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/DesignEditor.kt` around lines 772 - 780, Replace the verbose TextWatcher on binding.searchEditText with the doAfterTextChanged extension: remove the anonymous TextWatcher, add import androidx.core.widget.doAfterTextChanged, and call binding.searchEditText.doAfterTextChanged { adapter.filter(it?.toString() ?: "") } so adapter.filter is invoked after text changes; ensure you remove the unused override methods and keep the same behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/DesignEditor.kt`:
- Around line 772-780: Replace the verbose TextWatcher on binding.searchEditText
with the doAfterTextChanged extension: remove the anonymous TextWatcher, add
import androidx.core.widget.doAfterTextChanged, and call
binding.searchEditText.doAfterTextChanged { adapter.filter(it?.toString() ?: "")
} so adapter.filter is invoked after text changes; ensure you remove the unused
override methods and keep the same behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 301c45e0-ebf5-4545-95d2-4d8e20348e07
📒 Files selected for processing (1)
layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/DesignEditor.kt
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.
Description
Refactored the
BottomSheetDialogconfigurations inDesignEditor.ktto ensure proper rendering on larger screens (Tablets/DeX). By replacingSTATE_HALF_EXPANDEDwith a combination ofisFitToContents = trueandSTATE_EXPANDED, the dialogs now properly anchor to the bottom and wrap their content dynamically.Details
Tablet/DeX
document_5114392780575081850.mp4
Phone
Screen.Recording.2026-04-14.at.11.42.34.AM.mov
Ticket
ADFA-3071