Skip to content

Convert GutenbergKitEditorFragment from Java to Kotlin#22086

Merged
oguzkocer merged 14 commits intotrunkfrom
convert-gutenberg-kit-fragment-to-kotlin
Aug 1, 2025
Merged

Convert GutenbergKitEditorFragment from Java to Kotlin#22086
oguzkocer merged 14 commits intotrunkfrom
convert-gutenberg-kit-fragment-to-kotlin

Conversation

@oguzkocer
Copy link
Contributor

Summary

Complete conversion of GutenbergKitEditorFragment from Java to idiomatic Kotlin with systematic improvements while preserving functional compatibility.

Key Improvements

Safety & Reliability

  • Eliminated most !! operators and unsafe null assertions
  • Added proper null handling throughout critical code paths
  • Implemented type-safe settings accessors with reified generics

Code Quality & Maintainability

  • Applied Kotlin naming conventions (Hungarian notation → idiomatic Kotlin)
  • Simplified repetitive patterns with functional programming
  • Extracted complex configuration logic into dedicated methods
  • Added reusable extension functions for common UI operations

Performance

  • Replaced imperative loops with functional map operations
  • Implemented lazy sequences for collection processing
  • Reduced memory allocations and intermediate object creation

@oguzkocer oguzkocer added this to the 26.1 milestone Aug 1, 2025
@oguzkocer oguzkocer added the Gutenberg Editing and display of Gutenberg blocks. label Aug 1, 2025
@dangermattic
Copy link
Collaborator

dangermattic commented Aug 1, 2025

1 Warning
⚠️ This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.

Generated by 🚫 Danger

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Aug 1, 2025

WordPress📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
App NameWordPress WordPress
FlavorJalapeno
Build TypeDebug
Versionpr22086-a293fb3
Commita293fb3
Direct Downloadwordpress-prototype-build-pr22086-a293fb3.apk
Note: Google Login is not supported on these builds.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Aug 1, 2025

Jetpack📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
App NameJetpack Jetpack
FlavorJalapeno
Build TypeDebug
Versionpr22086-a293fb3
Commita293fb3
Direct Downloadjetpack-prototype-build-pr22086-a293fb3.apk
Note: Google Login is not supported on these builds.

@oguzkocer oguzkocer requested a review from dcalhoun August 1, 2025 06:23
Base automatically changed from move-gutenberg-kit-fragment-to-wordpress-module to trunk August 1, 2025 14:01
Copy link
Member

@dcalhoun dcalhoun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are significant stability and comprehensibility improvements. Thank you for completing this! 🙇🏻‍♂️

I stepped through each commit individually. Each of the commits a very logical and sound. Thank you for organizing the commit history so well.

I tested the experimental editor by performing documented test cases. As noted in an inline comment, the remote editor fails to load. We need to address that before merging this work.

Comment on lines +207 to +209
mEditorDragAndDropListener = requireActivityImplements<EditorDragAndDropListener>(activity)
mEditorImagePreviewListener = requireActivityImplements<EditorImagePreviewListener>(activity)
mEditorEditMediaListener = requireActivityImplements<EditorEditMediaListener>(activity)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will take additional investigation to confirm, but these are likely unused by GutenbergKit. In a future PR, we could potentially remove them (or implement them, if necessary). Similar to the numerous callbacks denoted with // Unused, no-op retained for the shared interface with Gutenberg.

To be clear: no action requested here now for this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's good feedback. I'll see what I can clean up from these in a subsequent PR.

@oguzkocer oguzkocer marked this pull request as ready for review August 1, 2025 15:30
oguzkocer and others added 14 commits August 1, 2025 11:35
…ragment

Fix compilation errors and warnings introduced during Java to Kotlin conversion:

Changes:
- Fix incorrect onAttach method signature (use Context instead of deprecated Activity)
- Correct GutenbergView listener setter method calls instead of property access
- Fix type signatures for getTitleAndContent return types (non-null vs nullable)
- Update ContentChangeListener interface method name (onContentChanged vs onChange)
- Fix EditorConfiguration builder with proper type casting and null safety
- Add proper @deprecated and @Suppress annotations for deprecated methods
- Fix nullable receiver warning with safe call operator for findViewById
- Add InterruptedException logging for better debugging
- Improve Kotlin idioms: property access, null safety, array handling
- Maintain exact functional compatibility with original Java implementation
Eliminate dangerous \!\! operators that could cause crashes:

Changes:
- Replace mGutenbergView\!\! access with safe calls (?.) or early returns
- Use local variable caching in onCreateView to avoid repeated null assertions
- Wrap unsafe access patterns with safe let blocks in onActivityResult and onDestroy
- Add proper null handling in getTitleAndContent method
- Convert onConfigurationChanged to use safe call operator
- Update all editor interaction methods (undo, redo, setContent, etc.) to use safe calls

This significantly reduces crash risk while maintaining exact functional behavior.
All methods now handle null GutenbergView gracefully without throwing NPE.
Move all GutenbergView setup logic into the .also block for better cohesion:

Changes:
- Consolidate view setup, listener configuration, and callbacks in single .also block
- Eliminate local gutenbergView variable and repeated references
- Keep all GutenbergView initialization logic together
- Improve code readability and reduce variable scope

This creates a more cohesive initialization pattern where all view setup
happens in one place immediately after view creation.
Move setEditorProgressBarVisibility(true) outside the .also block:

Changes:
- Keep only GutenbergView-specific setup inside .also block
- Move fragment-level UI logic (progress bar visibility) outside
- Maintain proper separation of concerns between view setup and fragment state

The .also block should only contain operations directly related to configuring
the GutenbergView instance, not broader fragment UI management.
Improve code readability by adopting Kotlin naming conventions for all properties:
- mGutenbergView → gutenbergView
- mHtmlModeEnabled → isHtmlModeEnabled (boolean with 'is' prefix)
- mEditorStarted → isEditorStarted
- mEditorDidMount → isEditorDidMount
- mRootView → rootView
- mHistoryChangeListener → historyChangeListener
- mFeaturedImageChangeListener → featuredImageChangeListener
- mOpenMediaLibraryListener → openMediaLibraryListener
- mOnLogJsExceptionListener → onLogJsExceptionListener
- mTextWatcher → textWatcher
- mSettings → settings (companion object property)

Changes:
- Eliminated Hungarian notation throughout the class
- Used 'is' prefix for boolean properties following Kotlin conventions
- Improved null safety with safe call operators where applicable
- Fixed companion object property assignment to avoid naming conflicts
- Maintained functional compatibility with existing interface contracts
Reduce code duplication and improve maintainability through pattern simplification:

Null Check Improvements:
- Replace repetitive if-null checks with idiomatic let functions
- Use method references for cleaner listener assignments
- Eliminate redundant null assertions with safe calls

Cast Pattern Improvements:
- Extract repetitive try-catch cast patterns into reusable helper function
- Add generic requireActivityImplements<T> method with reified types
- Improve error messages with automatic interface name detection

Configuration Building:
- Extract complex EditorConfiguration building into separate method
- Improve readability by isolating configuration logic
- Use more idiomatic Kotlin patterns (let, firstOrNull, etc.)
- Better variable naming and organization

Changes:
- historyChangeListener?.let(gutenbergView::setHistoryChangeListener)
- featuredImageChangeListener?.let(gutenbergView::setFeaturedImageChangeListener)
- openMediaLibraryListener?.let(gutenbergView::setOpenMediaLibraryListener)
- onLogJsExceptionListener?.let(gutenbergView::setLogJsExceptionListener)
- Added requireActivityImplements<T> generic helper method
- Extracted buildEditorConfiguration() method
- Improved string interpolation and collection operations
Replace unsafe casting patterns with type-safe extension functions to eliminate @Suppress("UNCHECKED_CAST") annotations throughout the codebase.

Key improvements:
- Added reified generic extension functions for Map<String, Any?> access
- getSetting<T>(key): Type-safe nullable accessor
- getSettingOrDefault<T>(key, default): Type-safe with fallback values
- getStringArray(key): Specialized accessor for string arrays with null filtering
- getWebViewGlobals(key): Specialized accessor for WebViewGlobal lists

buildEditorConfiguration() refactoring:
- Eliminated all unsafe casting with @Suppress annotations
- Used scope function 'run' for cleaner variable organization
- Improved null safety with proper default values
- Enhanced readability with type-safe accessors

Benefits:
- Compile-time type safety instead of runtime casting
- Cleaner code without suppression annotations
- Better error handling and null safety
- More maintainable and refactor-friendly settings access
Remove explicit unused parameter declaration from lambda expression for cleaner, more idiomatic Kotlin code.

Before: { _: GutenbergView? -> ... }
After:  { ... }

This follows Kotlin best practices where unused parameters should be omitted rather than explicitly marked with underscore.
Transform traditional for-loop into functional map operation for more idiomatic Kotlin code.

Improvements:
- Replace ArrayList initialization and manual loop with direct map operation
- Use destructuring declaration (url, mediaFile) for cleaner parameter access
- Eliminate mutable list in favor of immutable transformation
- Reduce code from 15 lines to 6 lines while maintaining identical functionality
- Remove unused metadata Bundle creation (videoPressGuid was not being used)

This follows functional programming principles by transforming data rather than mutating collections.
Replace eager collection operations with lazy sequence processing for better performance when filtering nullable string arrays.

Performance improvement:
- Use asSequence() for lazy evaluation instead of immediate collection creation
- Chain filterNotNull() -> toList() -> toTypedArray() operations lazily
- Reduces intermediate collection allocations for large arrays
- Maintains identical functionality while improving memory efficiency

This follows Kotlin best practices for collection processing when multiple transformations are chained together.
Create reusable extension function to simplify view visibility operations and improve code readability.

Extension function benefits:
- setVisibleOrGone(Boolean): Combines visibility logic into single method call
- Handles nullable View receivers safely with ?. operator
- Eliminates repetitive if-else visibility assignments
- Follows Kotlin idiom of extending platform classes with domain-specific operations

Usage improvement:
Before: view?.visibility = if (shown) View.VISIBLE else View.GONE
After:  view.setVisibleOrGone(shown)

This makes UI state management more expressive and reduces boilerplate code.
Resolves multiple Detekt rule violations to improve code maintainability
and readability while preserving existing functionality.

Changes:
- Extract nested logic from onActivityResult into helper functions
- Reduce return statements in getTitleAndContent from 3 to 2
- Fix line length violations by splitting long function declarations
- Maintain original behavior for file chooser handling and error cases
Co-authored-by: David Calhoun <github@davidcalhoun.me>
@oguzkocer oguzkocer force-pushed the convert-gutenberg-kit-fragment-to-kotlin branch from ab161e3 to a293fb3 Compare August 1, 2025 15:35
@oguzkocer oguzkocer enabled auto-merge (squash) August 1, 2025 15:36
@oguzkocer
Copy link
Contributor Author

@dcalhoun The force push is due to the rebase as squash merge strategy requires subsequent PRs to be rebased once the base PR is merged. In future PRs, I'll make sure to ping you if I make any major changes in the code during a force push. I try to avoid them as much as possible after I ping someone for review, but if I break that for whatever reason, I'll be sure to communicate it.

@oguzkocer oguzkocer requested a review from dcalhoun August 1, 2025 15:39
@sonarqubecloud
Copy link

sonarqubecloud bot commented Aug 1, 2025

Copy link
Member

@dcalhoun dcalhoun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The latest changes tested well for me. 🚀

@oguzkocer oguzkocer merged commit 232380a into trunk Aug 1, 2025
27 checks passed
@oguzkocer oguzkocer deleted the convert-gutenberg-kit-fragment-to-kotlin branch August 1, 2025 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Gutenberg Editing and display of Gutenberg blocks. Posting/Editing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants