secana/Forji
3
54
Fork
You've already forked Forji
7

feat: enable iPad and Mac Catalyst targets #68

Closed
pdurlej wants to merge 2 commits from pdurlej/Forji:codex/enable-ipad-mac-catalyst into main
pull from: pdurlej/Forji:codex/enable-ipad-mac-catalyst
merge into: secana:main
secana:main
Contributor
Copy link

Summary

This enables the Forji app target for iPad and Mac Catalyst builds with a minimal Xcode project change:

  • adds macosx to the supported platforms for the app target
  • enables SUPPORTS_MACCATALYST
  • changes the app target device family from iPhone-only to iPhone + iPad

The motivation is to make the existing iOS app usable from a desktop workflow as well, especially for pull request review/merge tasks where using Forji from a Mac can be more convenient than switching to a phone.

Notes

This PR intentionally does not redesign the UI for macOS and does not change any app code. It only exposes the existing app target to iPad and Mac Catalyst builds. I also have a local Mac Catalyst build available for private workflow testing if more feedback would be useful.

The Mac Catalyst build currently depends on an upstream Textual fix: https://github.com/gonzalezreal/textual/pull/60. Without that Textual change, Xcode reaches the Mac Catalyst destination but fails while compiling Textual because canImport(AppKit) selects AppKit APIs that are unavailable in Catalyst (NSFont, NSFontDescriptor, etc.). With that Textual PR applied locally to the SwiftPM checkout, the Forji Mac Catalyst build succeeds.

Verification

Tested on Xcode 26.5:

  • git diff --check
  • xcodebuild -project Forji/Forji.xcodeproj -scheme Forji -showdestinations
    • confirmed platform=macOS,variant=Mac Catalyst,name=My Mac is available
  • xcodebuild -project Forji/Forji.xcodeproj -scheme Forji -configuration Debug -destination 'platform=macOS,variant=Mac Catalyst,name=My Mac' ... build
    • expected failure without Textual PR #60, in Textual 0.3.1 AppKit/Catalyst guards
  • same build after applying https://github.com/gonzalezreal/textual/pull/60 to the resolved Textual checkout
    • passed
    • produced a signed Debug-maccatalyst/Forji.app
    • verified CFBundleSupportedPlatforms = MacOSX
    • codesign --verify --deep --strict passed

App Store exception

I grant Stefan Hausotte an irrevocable, worldwide, royalty-free license to use, sublicense, and distribute my contribution, including through Apple's App Store under the project's App Store exception.

## Summary This enables the `Forji` app target for iPad and Mac Catalyst builds with a minimal Xcode project change: - adds `macosx` to the supported platforms for the app target - enables `SUPPORTS_MACCATALYST` - changes the app target device family from iPhone-only to iPhone + iPad The motivation is to make the existing iOS app usable from a desktop workflow as well, especially for pull request review/merge tasks where using Forji from a Mac can be more convenient than switching to a phone. ## Notes This PR intentionally does not redesign the UI for macOS and does not change any app code. It only exposes the existing app target to iPad and Mac Catalyst builds. I also have a local Mac Catalyst build available for private workflow testing if more feedback would be useful. The Mac Catalyst build currently depends on an upstream Textual fix: https://github.com/gonzalezreal/textual/pull/60. Without that Textual change, Xcode reaches the Mac Catalyst destination but fails while compiling Textual because `canImport(AppKit)` selects AppKit APIs that are unavailable in Catalyst (`NSFont`, `NSFontDescriptor`, etc.). With that Textual PR applied locally to the SwiftPM checkout, the Forji Mac Catalyst build succeeds. ## Verification Tested on Xcode 26.5: - `git diff --check` - `xcodebuild -project Forji/Forji.xcodeproj -scheme Forji -showdestinations` - confirmed `platform=macOS,variant=Mac Catalyst,name=My Mac` is available - `xcodebuild -project Forji/Forji.xcodeproj -scheme Forji -configuration Debug -destination 'platform=macOS,variant=Mac Catalyst,name=My Mac' ... build` - expected failure without Textual PR #60, in Textual 0.3.1 AppKit/Catalyst guards - same build after applying https://github.com/gonzalezreal/textual/pull/60 to the resolved Textual checkout - passed - produced a signed `Debug-maccatalyst/Forji.app` - verified `CFBundleSupportedPlatforms = MacOSX` - `codesign --verify --deep --strict` passed ## App Store exception I grant Stefan Hausotte an irrevocable, worldwide, royalty-free license to use, sublicense, and distribute my contribution, including through Apple's App Store under the project's App Store exception.

Thanks a lot for putting this together, and for the thorough verification notes. I'm going to decline this one, though, for a mix of practical and project-scope reasons.

A few things I ran into while reviewing:

  • The Mac Catalyst target can't actually build against our pinned Textual (0.3.1). It needs the unmerged upstream PR #60, so a clean checkout would hit a compile failure on that destination, and CI (iOS Simulator
    only) wouldn't catch it.
  • The PR is described as project-settings-only, but it also rewrites KeychainManager and adds a keychain-access-groups entitlement. That's a security-sensitive area, and the iOS upgrade path (existing users keeping
    their stored tokens after the entitlement change) isn't verified.
  • iPad device family is enabled but the iPad UI itself isn't exercised anywhere in the testing.

Beyond the code, there's a project-scope reason too. Supporting more targets isn't free on the publishing side: each platform means extra work for App Store submission, including providing separate iPad screenshots
and a separate Mac review. I only want to add a platform once the UI genuinely handles it well on that form factor, and right now the app is designed and tested for iPhone. So enabling iPad and Catalyst before the
UI is adapted for them isn't the direction I want to take yet.

I really do appreciate the effort and the detail here, and the keychain LocalizedError improvement in particular was nice. It's just not something I want to merge right now. Thanks again!

Thanks a lot for putting this together, and for the thorough verification notes. I'm going to decline this one, though, for a mix of practical and project-scope reasons. A few things I ran into while reviewing: - The Mac Catalyst target can't actually build against our pinned Textual (0.3.1). It needs the unmerged upstream PR #60, so a clean checkout would hit a compile failure on that destination, and CI (iOS Simulator only) wouldn't catch it. - The PR is described as project-settings-only, but it also rewrites KeychainManager and adds a keychain-access-groups entitlement. That's a security-sensitive area, and the iOS upgrade path (existing users keeping their stored tokens after the entitlement change) isn't verified. - iPad device family is enabled but the iPad UI itself isn't exercised anywhere in the testing. Beyond the code, there's a project-scope reason too. Supporting more targets isn't free on the publishing side: each platform means extra work for App Store submission, including providing separate iPad screenshots and a separate Mac review. I only want to add a platform once the UI genuinely handles it well on that form factor, and right now the app is designed and tested for iPhone. So enabling iPad and Catalyst before the UI is adapted for them isn't the direction I want to take yet. I really do appreciate the effort and the detail here, and the keychain LocalizedError improvement in particular was nice. It's just not something I want to merge right now. Thanks again!
secana closed this pull request 2026年06月13日 14:02:22 +02:00
Author
Contributor
Copy link

No worries, thanks for taking a look @secana

Separate from this PR, I may build a small open-source macOS companion app for my own Forgejo review/merge workflow. It would likely live on GitHub and stay separate from Forji, so I don’t overload this project with my personal workflow needs.

Would you have any concerns with that, or any preference about how I should refer to Forji / avoid confusion with this app?

No worries, thanks for taking a look @secana Separate from this PR, I may build a small open-source macOS companion app for my own Forgejo review/merge workflow. It would likely live on GitHub and stay separate from Forji, so I don’t overload this project with my personal workflow needs. Would you have any concerns with that, or any preference about how I should refer to Forji / avoid confusion with this app?

Hi! Feel free to create your own project, but if you are willing to put in the work, maybe you can contribute real iPad support for Forji? If the UI is adapted to scale to an iPad and feels like a native app, not just an iPhone app that happens to run on it, I would happliy accept that.

I'm just uncomfortable with enabling the iPad support and having the publishing work, while knowing that Forji is not optimized for it.

Hi! Feel free to create your own project, but if you are willing to put in the work, maybe you can contribute real iPad support for Forji? If the UI is adapted to scale to an iPad and feels like a native app, not just an iPhone app that happens to run on it, I would happliy accept that. I'm just uncomfortable with enabling the iPad support and having the publishing work, while knowing that Forji is not optimized for it.

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
No reviewers
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
secana/Forji!68
Reference in a new issue
secana/Forji
No description provided.
Delete branch "pdurlej/Forji:codex/enable-ipad-mac-catalyst"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?