-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Add Conditional editor theme styles #25160
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 |
|
| App Name | WordPress | |
| Configuration | Release-Alpha | |
| Build Number | 30571 | |
| Version | PR #25160 | |
| Bundle ID | org.wordpress.alpha | |
| Commit | bc417dc | |
| Installation URL | 0h1hafhl0cem8 |
|
| App Name | Jetpack | |
| Configuration | Release-Alpha | |
| Build Number | 30571 | |
| Version | PR #25160 | |
| Bundle ID | com.jetpack.alpha | |
| Commit | bc417dc | |
| Installation URL | 1o58gg3jp2lr0 |
🤖 Build Failure AnalysisThis build has failures. Claude has analyzed them - check the build annotations for details. |
| async let currentUserTask = try await api.users.retrieveMeWithEditContext().data | ||
| async let activeThemeTask = try await api.themes.listWithEditContext( | ||
| params: ThemeListParams(status: .active) | ||
| ).data.first(where: { $0.status == .active }) |
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.
If one of the requests fails, the whole loadSiteInfoTask fails, which means we won't get any of the "site info". That's probably not something we'd want here?
| case .blockTheme: isBlockTheme | ||
| case .plugins: apiRoot.hasRoute(route: "/wp/v2/plugins") | ||
| case .applicationPasswordExtras: apiRoot.hasRoute(route: "/application-password-extras/v1/admin-ajax") | ||
| } |
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.
Nitpick: using the urlResolver property in WordPressAPI to find the URL should work on both self-hosted and WP.com sites, so we can merge the two similar branches into one.
| let key = blog.locallyUniqueId + "-capabilities" | ||
|
|
||
| // Don't allow more than one concurrent invalidation | ||
| if self.prefetchTasks[key] != nil { |
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.
Unrelated to this PR: you can use OSAllocatedUnfairLock in LockingHashMap, which is Sendable, to get rid of the @unchecked declaration, which means it's impossible (as long as we continue to have the Swift compiler check in place) to forget use the lock.withLock.
|
Noting some early testing results:
Theme styles settings reset
theme-styles-settings.MP4 |
36bd1c1 to
cfc334e
Compare
cfc334e to
bc417dc
Compare
|
|
Version |





Summary
WordPressClientfeature detection and caching behaviour – there's more to do here, but that'll be a future PR.Changes
WordPressClientFeature DetectionWordPressClientAPIprotocol to abstract WordPress API access for testingsupports(_:forSiteId:)method to check API route availability for featuresTaskto avoid redundant API calls. In the future, we'll want a way to invalidate this, but we'll do that later.Editor Capability Fetching
EditorDependencyManagernow fetches and caches editor capabilities on My Site loadGutenbergSettingsstores feature support per-blog usingWordPressClient.Feature.stringValueas stable keysTesting Instructions
cpt.wpmt.coto the app. Ensure the editor loads as expected. Ensure that theme styles toggle cannot be activated in Site Settings.jetpack.wpmt.coto the app. Ensure the editor loads. Ensure that theme styles toggle can be activated.