Repository navigation
Visual Editor - Settings Screen Presentation Update - #6987
Open
voyagerfan wants to merge 17 commits into
Open
voyagerfan wants to merge 17 commits into
voyagerfan wants to merge 17 commits into
Conversation
voyagerfan
requested review from
Williamrai,
cooltey and
dbrant
as code owners
October 9, 2026 21:51
voyagerfan
changed the title
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new Compose flow performs preference and analytics side effects directly in UI code instead of using the required backing ViewModel.
1 open finding
What changed in this PR
Replaces the visual-editor preference dialog with a dedicated Compose settings screen.
Changes:
- Adds the new settings activity and manifest entry.
- Saves editor selection immediately and updates analytics.
- Simplifies the remaining editor-choice dialog.
| File | Description |
|---|---|
strings.xml |
Removes the obsolete Save label. |
values-qq/strings.xml |
Removes obsolete localization guidance. |
VisualEditorSettingsActivity.kt |
Adds the Compose settings screen. |
SettingsPreferenceLoader.kt |
Opens the screen and logs interaction. |
PageActivity.kt |
Adapts to the simplified dialog API. |
EditorChoiceDialog.kt |
Shares editor-choice UI with the screen. |
AndroidManifest.xml |
Registers the new activity. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+116
to
+121
| if (isInSettingsScreen && Prefs.editorModeChoice != editorChoice) { | ||
| Prefs.editorModeChoice = editorChoice | ||
| instrument.submitInteraction( | ||
| action = "click", | ||
| actionSource = "settings", | ||
| elementId = if (editorChoice == EDITOR_CHOICE_VE) "visual_editing" else "source_editing" |
Collaborator
Author
There was a problem hiding this comment.
Keeping as is for now. The settings screen only deals with 1 pref. Hoisting the side effects would create a second callback alongside onConfirm.
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
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.
What does this do?
This PR updates the settings screen per the design provided in T434149 by replacing the dialog pop up with a dedicated settings screen.
Why is this needed?
To align with the layout/formatting with existing settings screens
Phabricator:
https://phabricator.wikimedia.org/T434149