-
Notifications
You must be signed in to change notification settings - Fork 1.1k
refactor: Use GutenbergKit configuration builder #24662
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
Conversation
Generated by 🚫 Danger |
WordPress/Classes/ViewRelated/NewGutenberg/NewGutenbergViewController.swift
Outdated
Show resolved
Hide resolved
125229e
to
518ebe3
Compare
|
c69fd9d
to
4a258ed
Compare
|
App Name | WordPress | |
Configuration | Release-Alpha | |
Build Number | 29037 | |
Version | PR #24662 | |
Bundle ID | org.wordpress.alpha | |
Commit | 629b028 | |
Installation URL | 6bv21r72lunp8 |
|
App Name | Jetpack | |
Configuration | Release-Alpha | |
Build Number | 29037 | |
Version | PR #24662 | |
Bundle ID | com.jetpack.alpha | |
Commit | 629b028 | |
Installation URL | 58ufb2tsqprq8 |
4a258ed
to
49126f1
Compare
Adopt the latest patterns to embrace immutable configuration.
49126f1
to
cfc5d97
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jkmassel when testing the prototype build, I noted the theme styles were not loaded when I had the Experimental Block Editor Styles feature enabled. When I built this branch locally in Xcode 26 RC, I encountered a crash (image below) in the editor settings request logic. I imagine the crash/failed request explains the absence of theme styles (and rest of the editor settings).
Do you encounter this? What might be occurring?
// We can't use wordpress-rs branches nor commits here. Only tags work. | ||
.package(url: "https://github.com/Automattic/wordpress-rs", revision: "alpha-20250901"), | ||
.package(url: "https://github.com/wordpress-mobile/GutenbergKit", from: "0.8.0"), | ||
.package(url: "https://github.com/wordpress-mobile/GutenbergKit", revision: "f3e64e4d7edc60a5c09b84e9421aa80516231414"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before we merge this, we should merge the accompanying GBK PR, publish a GBK prerelease (e.g., v0.8.1-alpha.1
), and integrate the version number here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It appears this now duplicates the extension found in the view controller. I believe we should consolidate these.
The extension in the view controller was relocated in 518ebe3 — an older commit now detached from this branch. At some point after that, a commit was forced pushed that reintroduced the extension in the view controller, likely during a conflict resolution.
However, the extension in the view controller appears more accurate and up-to-date, handling nuanced situations like preferring application passwords over OAuth tokens.
We should use the copy in the view controller. We can either leave it there and remove this file, or update this file and remove the copy in the view controller.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in 60b848e – keeping the logic in a ViewController file can make it hard to find, so I've moved all the logic over to EditorConfiguration+Blog
, and unified the calls to it.
0817212
to
45ba29d
Compare
The crash is addressed in 45ba29d – it mostly shouldn't happen in production, that's the Core Data thread sanitizer complaining that we're accessing a Core Data object on the wrong thread. I reworked the initialization of that service to resolve it. The root cause is likely that I removed I'd be a little bit surprised if this was the cause of the missing theme styles, but it's possible. |
|
Description
Adopt the latest patterns for embracing immutable configuration introduced in wordpress-mobile/GutenbergKit#146.
Testing instructions
Smoke test editor functionality dependent upon configuration—theme styles, media uploads, remote editor (plugins), etc.