-
Notifications
You must be signed in to change notification settings - Fork 1.1k
refactor: Fetch and cache GutenbergKit editor settings #24417
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
Changes from all commits
3179db1
dfce8e6
1952cc9
8ebe71e
b3b2129
f9231fa
17d5798
de5e535
e5222b4
45c644c
4ce5fd9
0e4ba97
c379b1a
e19b669
09ffd57
676f0ff
bf7fcff
fe5ff15
bfa177e
8f12133
acd2567
e8a892c
487ede4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| import Foundation | ||
| import CoreData | ||
|
|
||
| extension Blog { | ||
| private static let rawBlockEditorSettingsKey = "rawBlockEditorSettings" | ||
|
|
||
| /// Stores the raw block editor settings dictionary | ||
| var rawBlockEditorSettings: [String: Any]? { | ||
| get { | ||
| return getOptionValue(Self.rawBlockEditorSettingsKey) as? [String: Any] | ||
| } | ||
| set { | ||
| setValue(newValue, forOption: Self.rawBlockEditorSettingsKey) | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| import Foundation | ||
| import WordPressKit | ||
| import WordPressShared | ||
|
|
||
| class RawBlockEditorSettingsService { | ||
| private let blog: Blog | ||
| private let remoteAPI: WordPressOrgRestApi | ||
| private var isRefreshing: Bool = false | ||
|
|
||
| init?(blog: Blog) { | ||
| guard let remoteAPI = WordPressOrgRestApi(blog: blog) else { | ||
| return nil | ||
| } | ||
|
|
||
| self.blog = blog | ||
| self.remoteAPI = remoteAPI | ||
| } | ||
|
|
||
| @MainActor | ||
| private func fetchSettingsFromAPI() async throws -> [String: Any] { | ||
| let result = await self.remoteAPI.get(path: "/wp-block-editor/v1/settings") | ||
| switch result { | ||
| case .success(let response): | ||
| guard let dictionary = response as? [String: Any] else { | ||
| throw NSError(domain: "RawBlockEditorSettingsService", code: 1, userInfo: [NSLocalizedDescriptionKey: "Invalid response format"]) | ||
| } | ||
| blog.rawBlockEditorSettings = dictionary | ||
| return dictionary | ||
| case .failure(let error): | ||
| throw error | ||
| } | ||
| } | ||
|
|
||
| @MainActor | ||
| func fetchSettings() async throws -> [String: Any] { | ||
| // Start a background refresh if not already refreshing | ||
| if !isRefreshing { | ||
| isRefreshing = true | ||
| Task { | ||
| do { | ||
| _ = try await fetchSettingsFromAPI() | ||
| } catch { | ||
| DDLogError("Error refreshing block editor settings: \(error)") | ||
| } | ||
| isRefreshing = false | ||
| } | ||
| } | ||
|
|
||
| // Return cached settings if available | ||
| if let cachedSettings = blog.rawBlockEditorSettings { | ||
| return cachedSettings | ||
| } | ||
|
|
||
| // If no cache and no background refresh in progress, fetch synchronously | ||
| if !isRefreshing { | ||
| return try await fetchSettingsFromAPI() | ||
| } | ||
|
|
||
| // If we're here, it means a background refresh is in progress | ||
| // Wait for it to complete and return the cached result | ||
| while isRefreshing { | ||
| try await Task.sleep(nanoseconds: 100_000_000) // 100ms | ||
| if let cachedSettings = blog.rawBlockEditorSettings { | ||
| return cachedSettings | ||
| } | ||
| } | ||
|
|
||
| // If we still don't have settings after the refresh completed, throw an error | ||
| throw NSError(domain: "RawBlockEditorSettingsService", code: 2, userInfo: [NSLocalizedDescriptionKey: "Failed to fetch block editor settings"]) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -65,9 +65,16 @@ class NewGutenbergViewController: UIViewController, PostEditor, PublishingEditor | |
| BlockEditorSettingsService(blog: post.blog, coreDataStack: ContextManager.shared) | ||
| }() | ||
|
|
||
| // New service for fetching raw block editor settings | ||
| lazy var rawBlockEditorSettingsService: RawBlockEditorSettingsService? = { | ||
| return RawBlockEditorSettingsService(blog: post.blog) | ||
| }() | ||
|
|
||
| // MARK: - GutenbergKit | ||
|
|
||
| private let editorViewController: GutenbergKit.EditorViewController | ||
| private var editorViewController: GutenbergKit.EditorViewController | ||
| private var activityIndicator: UIActivityIndicatorView? | ||
| private var hasEditorStarted = false | ||
|
|
||
| lazy var autosaver = Autosaver() { | ||
| self.performAutoSave() | ||
|
|
@@ -202,7 +209,11 @@ class NewGutenbergViewController: UIViewController, PostEditor, PublishingEditor | |
| configureNavigationBar() | ||
| refreshInterface() | ||
|
|
||
| fetchBlockSettings() | ||
| // Show activity indicator while fetching settings | ||
| showActivityIndicator() | ||
|
|
||
| // Fetch block editor settings | ||
| fetchBlockEditorSettings() | ||
|
|
||
| // TODO: reimplement | ||
| // service?.syncJetpackSettingsForBlog(post.blog, success: { [weak self] in | ||
|
|
@@ -316,6 +327,70 @@ class NewGutenbergViewController: UIViewController, PostEditor, PublishingEditor | |
| WordPressAppDelegate.crashLogging?.logJavaScriptException(exception, callback: callback) | ||
| } | ||
| } | ||
|
|
||
| // MARK: - Activity Indicator | ||
|
|
||
| private func showActivityIndicator() { | ||
| let indicator = UIActivityIndicatorView(style: .large) | ||
| indicator.color = .gray | ||
| indicator.translatesAutoresizingMaskIntoConstraints = false | ||
| view.addSubview(indicator) | ||
|
|
||
| NSLayoutConstraint.activate([ | ||
| indicator.centerXAnchor.constraint(equalTo: view.centerXAnchor), | ||
| indicator.centerYAnchor.constraint(equalTo: view.centerYAnchor) | ||
| ]) | ||
|
|
||
| indicator.startAnimating() | ||
| self.activityIndicator = indicator | ||
| } | ||
|
|
||
| private func hideActivityIndicator() { | ||
| activityIndicator?.stopAnimating() | ||
| activityIndicator?.removeFromSuperview() | ||
| activityIndicator = nil | ||
| } | ||
|
|
||
| // MARK: - Block Editor Settings | ||
|
|
||
| private func fetchBlockEditorSettings() { | ||
dcalhoun marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| guard let service = rawBlockEditorSettingsService else { | ||
| startEditor() | ||
| return | ||
| } | ||
|
|
||
| Task { @MainActor in | ||
| // Start the editor with default settings after 3 seconds | ||
| let timeoutTask = Task { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a clever way to address this – I think ideally we'd just use the |
||
| try await Task.sleep(nanoseconds: 3_000_000_000) // 3 seconds | ||
| if !Task.isCancelled { | ||
| startEditor() | ||
| } | ||
| } | ||
|
|
||
| do { | ||
| let settings = try await service.fetchSettings() | ||
| timeoutTask.cancel() | ||
| startEditor(with: settings) | ||
| } catch { | ||
| timeoutTask.cancel() | ||
| DDLogError("Error fetching block editor settings: \(error)") | ||
| startEditor() | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private func startEditor(with settings: [String: Any]? = nil) { | ||
| guard !hasEditorStarted else { return } | ||
| hasEditorStarted = true | ||
|
|
||
| if let settings { | ||
| var updatedConfig = self.editorViewController.configuration | ||
| updatedConfig.updateEditorSettings(settings) | ||
| self.editorViewController.updateConfiguration(updatedConfig) | ||
| } | ||
| self.editorViewController.startEditorSetup() | ||
| } | ||
| } | ||
|
|
||
| extension NewGutenbergViewController: GutenbergKit.EditorViewControllerDelegate { | ||
|
|
@@ -327,6 +402,7 @@ extension NewGutenbergViewController: GutenbergKit.EditorViewControllerDelegate | |
| // is still reflecting the actual startup time of the editor | ||
| editorSession.start() | ||
| } | ||
| self.hideActivityIndicator() | ||
| } | ||
|
|
||
| func editor(_ viewContoller: GutenbergKit.EditorViewController, didDisplayInitialContent content: String) { | ||
|
|
||
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.
What happens if we've never fetched the settings and the network connection fails?
In my testing it seems like it'll just show a spinner forever?
If we don't have locally cached settings and the network request fails we should probably:
I could see an argument for showing some kind of editor with no remote settings, but I don't know how feasible that is?
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.
Your experience is surprising. These changes were built to accomplish what you describe. When I block the network request (via Charles Proxy) for the editor settings, the editor continues to load with default editor settings after a moment.
How are you causing the network connection failure in your testing?
The screen recording below showcases both the default settings and site-specific settings.
Screen.Recording.2025-04-16.at.16.44.12.mov
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.
In fact, the situation you describe is a part of the testing instructions in the PR description. 😄 I'm perplexed as to why you may be experiencing a different outcome.