Skip to content
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

Merged
merged 15 commits into from
Jul 5, 2024

Conversation

kean
Copy link
Contributor

@kean kean commented Jun 27, 2024

I tested an original PR and there was a small issue with WordPressKit not being included in the Jetpack app. I fixed it, and also followed Tony's suggested next steps by importing WordPressShared and WordPressUI 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

  • Merge trunk and fix conflicts
  • Made a couple of PRs removing dependencies where I could: Remove Quick and OCMock dependencies WordPress-iOS-Shared#359, Remove WPImageSource and OHHTTPStubs dependency WordPress-iOS-Shared#360.
  • Add SPM support in WordPressUI Add SPM support WordPressUI-iOS#158 by temporarily removing the broken SwiftLint dependency. There will be no need for it once the framework is moved to the monorepo.
  • Add SPM support to WordPressShared – same change.
  • Integrate WordPressShared and WordPressUI to WordPress using SPM
  • Fix issues with WordPressKit being set to “Embed & Sign” in multiple targets, including WordPressAuthentificator and others, which was leading to it being duplicated multiple times. Embed WordPressKit 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.
  • Disable “Unused Parameter” warnings for WPKit – there were hundreds of them, most false positives
  • Rewrite LoginFacadeTests tests without Specta and Expect in order to remove them (it was needed because without the custom scripts I mentioned earlier, it was no longer working)
  • Re-shuffle some of the code and imports in WordPressAuthentificator and WordPressKit to make sure it doesnt' re-export WordPressUI and other modules in its Objective-C auto-generated module header.
  • Add missing @executable_path/../../Frameworks to the share extensions – apparently it got deleted at some point

Next Steps

  • Remove remaining CocoaPods dependencies (question mark – Gutenberg)
  • Add a single Package.swift file that will define all the targets in the app and all their dependencies (stop using Xcode UI for that)
  • Add WordPressShared and WordPressUI to the monorepo
  • Try to fix an issue with duplicated module bundles https://forums.developer.apple.com/forums/thread/749265 (not critical as it's only adding a few MBs, uncompressed)
  • (Future) Update WordPressAuthentificator to use the new Gravatar package and install it as a Swift package – looks like the team integrating it missed it

To test:

  • Test that the TestFlight build doesn't crash
  • Verify the login flows

Regression Notes

  1. Potential unintended areas of impact: everything

  2. What I did to test those areas of impact (or what existing automated tests I relied on)

  3. What automated tests I added (or what prevented me from doing so)

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding unit tests for my changes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

Testing checklist:

  • WordPress.com sites and self-hosted Jetpack sites.
  • Portrait and landscape orientations.
  • Light and dark modes.
  • Fonts: Larger, smaller and bold text.
  • High contrast.
  • VoiceOver.
  • Languages with large words or with letters/accents not frequently used in English.
  • Right-to-left languages. (Even if translation isn’t complete, formatting should still respect the right-to-left layout)
  • iPhone and iPad.
  • Multi-tasking: Split view and Slide over. (iPad)

@kean kean changed the base branch from trunk to tonyli-xcode-target-authenticator June 27, 2024 19:55
@kean kean changed the title Integrate WordPressKit and WordPressAuthentificator using Xcode targets Update WordPressKit and WordPressAuthentificator integration Jun 27, 2024
@kean kean changed the title Update WordPressKit and WordPressAuthentificator integration Update WordPressKit and WordPressAuthentificator setup Jun 27, 2024
@dangermattic
Copy link
Collaborator

dangermattic commented Jun 27, 2024

2 Warnings
⚠️ View files have been modified, but no screenshot or video is included in the pull request. Consider adding some for clarity.
⚠️ This PR is larger than 500 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.

Generated by 🚫 Danger

@kean kean added the Tooling Build, Release, and Validation Tools label Jun 27, 2024
@wpmobilebot
Copy link
Contributor

wpmobilebot commented Jun 27, 2024

WordPress Alpha📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.
App NameWordPress Alpha WordPress Alpha
ConfigurationRelease-Alpha
Build Numberpr23392-68cf81b
Version25.1
Bundle IDorg.wordpress.alpha
Commit68cf81b
App Center BuildWPiOS - One-Offs #10236
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Jun 27, 2024

Jetpack Alpha📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.
App NameJetpack Alpha Jetpack Alpha
ConfigurationRelease-Alpha
Build Numberpr23392-68cf81b
Version25.1
Bundle IDcom.jetpack.alpha
Commit68cf81b
App Center Buildjetpack-installable-builds #9285
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@kean kean changed the base branch from tonyli-xcode-target-authenticator to task/xcode-targets-merge-trunk June 27, 2024 21:15

// MARK: - RotationAwareNavigationViewController
//
public class RotationAwareNavigationViewController: UINavigationController {
Copy link
Contributor Author

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()
Copy link
Contributor Author

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.

@kean kean requested a review from jkmassel June 27, 2024 21:55
@jkmassel jkmassel merged commit 1cf0af0 into task/xcode-targets-merge-trunk Jul 5, 2024
24 checks passed
@jkmassel jkmassel deleted the task/xcode-targets-wp branch July 5, 2024 19:25
jkmassel added a commit that referenced this pull request Jul 5, 2024
* 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]>
@kean kean restored the task/xcode-targets-wp branch July 5, 2024 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Tooling Build, Release, and Validation Tools
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants