Skip to content

ADFA-4072: treat system back button as Cancel in settings dialogs#1372

Open
Daniel-ADFA wants to merge 1 commit into
stagefrom
ADFA-4072
Open

ADFA-4072: treat system back button as Cancel in settings dialogs#1372
Daniel-ADFA wants to merge 1 commit into
stagefrom
ADFA-4072

Conversation

@Daniel-ADFA
Copy link
Copy Markdown
Contributor

No description provided.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jun 5, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fe31c000-b8b6-4bd0-9ca9-6ee76449bc32

📥 Commits

Reviewing files that changed from the base of the PR and between 3640116 and 9e7f116.

📒 Files selected for processing (5)
  • app/src/main/java/com/itsaky/androidide/preferences/editorPrefExts.kt
  • app/src/main/java/com/itsaky/androidide/preferences/generalPrefExts.kt
  • app/src/main/java/com/itsaky/androidide/preferences/xmlPrefExts.kt
  • preferences/src/main/java/com/itsaky/androidide/preferences/ChoiceBasedDialogPreference.kt
  • preferences/src/main/java/com/itsaky/androidide/preferences/DialogPreference.kt
💤 Files with no reviewable changes (3)
  • app/src/main/java/com/itsaky/androidide/preferences/xmlPrefExts.kt
  • app/src/main/java/com/itsaky/androidide/preferences/generalPrefExts.kt
  • app/src/main/java/com/itsaky/androidide/preferences/editorPrefExts.kt

📝 Walkthrough

Release Notes - ADFA-4072: Treat system back button as Cancel in settings dialogs

Changes

  • Default behavior: DialogPreference.dialogCancellable now defaults to true (changed from false)
  • New cancellation hook: Added onDialogCancelled() method to DialogPreference that is invoked when dialogs are cancelled via system back button or other framework-triggered cancellations
  • Unified cancellation handling: ChoiceBasedDialogPreference now forwards system cancellations to onChoicesCancelled(), ensuring consistent behavior with negative button cancellations
  • Removed redundant overrides: Eliminated explicit dialogCancellable = true overrides from TabSize, UiMode, and EmptyElementsBehavior classes (now inherit the new default)

Risks & Best Practice Violations

⚠️ Breaking Change: The default dialogCancellable value changing from false to true is a significant behavioral shift that affects all DialogPreference subclasses. Any preference dialogs that did not explicitly set this property will now be cancellable by the system back button, which may have unintended consequences in existing implementations.

⚠️ Inconsistent Cancellation Paths: EditTextPreference and NumberInputEditTextPreference do not override onDialogCancelled(). This creates a gap where:

  • Negative button click: Dialog is dismissed with no special handling
  • System back button: Dialog is dismissed via the empty onDialogCancelled() hook

This inconsistency could lead to subtle bugs or unexpected behavior differences depending on how the user dismisses the dialog.

⚠️ Incomplete Migration: While ChoiceBasedDialogPreference properly implements onDialogCancelled(), other DialogPreference subclasses (EditTextPreference, NumberInputEditTextPreference, and the TextSize preference in editor settings) don't have corresponding implementations, making the new cancellation path incomplete.

Walkthrough

The PR establishes a centralized dialog cancellation handler in the base DialogPreference class, changes its default cancellability to true, and removes now-redundant overrides from preference extensions. Subclasses can override the new onDialogCancelled hook to handle back-button and system cancellations separately from action-button actions.

Changes

Dialog Cancellation Refactor

Layer / File(s) Summary
Base dialog cancellation infrastructure
preferences/src/main/java/com/itsaky/androidide/preferences/DialogPreference.kt
DialogPreference defaults dialogCancellable to true, registers an onCancelListener during dialog creation, and introduces a new protected open fun onDialogCancelled(preference: Preference) hook method for subclasses to override.
Choice-based dialog cancellation
preferences/src/main/java/com/itsaky/androidide/preferences/ChoiceBasedDialogPreference.kt
ChoiceBasedDialogPreference implements onDialogCancelled to forward cancellation events to its onChoicesCancelled(preference) callback.
Remove redundant dialogCancellable overrides
app/src/main/java/com/itsaky/androidide/preferences/editorPrefExts.kt, app/src/main/java/com/itsaky/androidide/preferences/generalPrefExts.kt, app/src/main/java/com/itsaky/androidide/preferences/xmlPrefExts.kt
Extension preference classes (TabSize, UiMode, EmptyElementsBehavior) remove explicit dialogCancellable = true overrides and update parcelization imports; cancellation behavior now relies on the base class default.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • jomen-adfa

Poem

🐰 A dialog that cancels with grace,
No need to override in every place,
The base now handles what comes back,
While cleanups trim the coding stack.
The hook is ready to receive,
A better pattern we now weave! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning No description was provided by the author, which fails this check as there is no content to evaluate against the changeset. Add a pull request description explaining the motivation for making dialogs cancellable by default and how the cancellation handling works.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: treating system back button as Cancel in settings dialogs, which is supported by the changes that make dialogs cancellable by default and add cancellation handling.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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 ADFA-4072

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Daniel-ADFA Daniel-ADFA requested a review from a team June 5, 2026 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants