-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Update WordPressKit and WordPressAuthentificator setup #23392
Update WordPressKit and WordPressAuthentificator setup #23392
Conversation
Generated by 🚫 Danger |
📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.
|
📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.
|
|
||
// MARK: - RotationAwareNavigationViewController | ||
// | ||
public class RotationAwareNavigationViewController: UINavigationController { |
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.
I moved it from WordPressUI
to make sure WordPressAuthentificator
doesn't re-exposes WordPressUI
in its auto-generated header when creating an Objective-C module. I made some changes to remove its uses from Objective-C, that I kept, but then realized that there were a ton more usages from its test target, so I had to fix it.
Evidently, you can't include a Swift module in headers for an Objective-C module.
@objc optional func secondaryButtonPressed() | ||
@objc optional func tertiaryButtonPressed() | ||
func secondaryButtonPressed() | ||
func tertiaryButtonPressed() |
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's a safe change because it never used dynamically from Objective-C (think respondsToSelector:
to dynamically configure the UI.
* Optimize TopTotalsCell to add rows only when the cell loads TopTotalsCell was calling addRows on every configuration of cell which in turn created and added a hierarchy of UIStackView-based views. Optimizing TopTotalsCell to only add rows once and then make manipulations on existing rows. * Optimize CountriesCell to add rows only when the cell loads * Do not track StatsTraffic tableView scrolling * Update RELEASE-NOTES.txt * Update TopTotalsCell to use setNeedsLayout for more efficiency * Update CountriesCell to use setNeedsLayout for more efficiency * Update RELEASE-NOTES * Move additional checks for adding default rows into the extension * Fix rare crash in GutenbergWebViewController * Update release notes * Remove force layout calls when setting subtitle visibility These calls were added together with dynamic type support, however, they slow down layout process of the cell * Make maximum content size category smaller for stats cell subtitles * Create StatsRowsCell with default child stack view rows and ability to configure more * Put analyticsTracker back since it's used by JetpackBanner * JPBackground as png * Move JPBackground to AppImage specific to the Jetpack app * Remove Stories related files * Remove unused site creation icons * Reduce rppreview size * Replace JPBackground with tiny-fied icons * Remove custom fonts used by Kanvas * Remove remaining Kanvas related code * Remove Kanvas related code * Remove StoryEditor * Replace remaining Kanvas usages * Remove StoriesIntroViewController * Remove Kanvas pod * Update rubocop.yml * Add unique identifier to file downloads rows (#23310) File Downloads data can be identical which can result in a rare duplicate diffable data source identifiers crash. Pass a unique identifier to ensure that each file downloads row is treated as unique. * Support editing media metadata via XML-RPC #809 (#23316) * Support media metadata editing for XML-RPC connected self-hosted sites - Updated WordPressKit supports editing title, description, and caption of the media via XML-RPC - XML-RPC API doesn't support editing alt-text * Support editing media metadata via XMLRPC in MediaService Media is a type of a post therefore "wp.editPost" can be used to edit media metadata. Note that alternative text cannot be edited due to lack of XML-RPC support https://core.trac.wordpress.org/ticket/58582 * Update RELEASE-NOTES.txt * Fix warnings in MemoryCache * Fix warnings in CachedAsyncImage * Fix more warnings * Remove deprecated in JetpackBrandingVisibility.enabled * Remove AlamofireNetworkActivityIndicator (#23385) * Merge 25.1 release finalization (#23391) * Fix announcement card keep showing up after tapping Done (#23384) * Update app translations – `Localizable.strings` * Update WordPress metadata translations * Update Jetpack metadata translations * Bump version number --------- Co-authored-by: David Christiandy <[email protected]> * Update WordPressKit and WordPressAuthentificator setup (#23392) * Install WordPressUI using SPM * Remove WordPressShared from Podfile * Add WordPressShared using SPM * Fix WordPress compilation * Fix WordPressKit being embeded in the wrong targets * Disable some warnings in WordPressKit * Remove redundant manual linker flags * Fix WordPressKit tests * Fix WordPressAuthentificator tests * Remove Specta and Expecta from Podfile * Fix WordPressAuthentificator tests by temporary disabling LoginFacadeTests * Update WordPressAuthenticator so that it could be compliled as an ObjC module again * Rewrite LoginFacadeTests * Fix WordPressTests * Add missing executable_path/../../Frameworks in the share extensions --------- Co-authored-by: Povilas Staskus <[email protected]> Co-authored-by: Jeremy Massel <[email protected]> Co-authored-by: WordPress Mobile Bot Account <[email protected]> Co-authored-by: David Christiandy <[email protected]>
I tested an original PR and there was a small issue with
WordPressKit
not being included in theJetpack
app. I fixed it, and also followed Tony's suggested next steps by importingWordPressShared
andWordPressUI
using SPM to get rid of the custom CocoaPods scripts.Tony's PR was a bit behind, so I merged trunk in a separate PR to make it easier to review: #23393.
Changes
WordPressAuthentificator
and others, which was leading to it being duplicated multiple times. EmbedWordPressKit
in the app target itself. You can’t embed one dynamic framework framework into another dynamic framework on iOS – it works only on macOS. They have to go in the app.LoginFacadeTests
tests withoutSpecta
andExpect
in order to remove them (it was needed because without the custom scripts I mentioned earlier, it was no longer working)WordPressAuthentificator
andWordPressKit
to make sure it doesnt' re-exportWordPressUI
and other modules in its Objective-C auto-generated module header.@executable_path/../../Frameworks
to the share extensions – apparently it got deleted at some pointNext Steps
Package.swift
file that will define all the targets in the app and all their dependencies (stop using Xcode UI for that)WordPressShared
andWordPressUI
to the monorepoWordPressAuthentificator
to use the newGravatar
package and install it as a Swift package – looks like the team integrating it missed itTo test:
Regression Notes
Potential unintended areas of impact: everything
What I did to test those areas of impact (or what existing automated tests I relied on)
What automated tests I added (or what prevented me from doing so)
PR submission checklist:
RELEASE-NOTES.txt
if necessary.Testing checklist: