Repository navigation
Add an iOS app clip sample - #538
Conversation
redbrogdon
left a comment
redbrogdon
left a comment
There was a problem hiding this comment.
I'm not sure if this repo is the right place for this. I get that the goal for the sample is that it's set up to build out of the box as an AppClip, but what's the content? Like, if you diffed this against the Counter app from flutter create, what's going to be different?
The reason I ask is that if it's not that much code, you might be better off just sticking with the guide on flutter.dev by itself.
|
|
||
| version: | ||
| revision: 7e480edc7e531c400aff4f08afeebe04c0f63e2e | ||
| channel: cupertino-icon-5 |
There was a problem hiding this comment.
Do we have new channels?
There was a problem hiding this comment.
doh, fixed it to what it should have been.
|
Ya, it's definitely debatable. The goal is to have an out of the box thing that builds like you're saying so we can test. Also a known working case that users can validate against after following through the guide and perhaps running into problems. It's a way to bisect the problem. The difference with a blank flutter created template is going to be mostly in Runner.xcodeproj file. It's a bit like the prebuilt iOS host app example in add-to-app. There isn't too much code. It's A) to let you spin up an add-to-app example quickly to see it running, and to prove that it works, and B) to double check your own integration steps (which is also manual on flutter.dev). In that case too, a lot of the diff vs a blank Xcode project is in the Xcode project hook-up itself. |
|
Gentle ping |
redbrogdon
left a comment
redbrogdon
left a comment
There was a problem hiding this comment.
Sounds good to land it here. I've added a few nitpicks, but nothing significant. 😄
| } | ||
|
|
||
| class MyApp extends StatelessWidget { | ||
| // This widget is the root of your application. |
There was a problem hiding this comment.
I would recommend cutting all of these comments rom the template. Also, consider stripping this down to just the plugin output and some kind of graphic (like a spinning Flutter logo or something).
| import 'package:ios_app_clip/main.dart'; | ||
|
|
||
| void main() { | ||
| testWidgets('Counter increments smoke test', (WidgetTester tester) async { |
There was a problem hiding this comment.
If you revise the main page of the app, feel free to replace this with a stub test. It does need to have one, though, and get listed in /tool/travis_flutter_script.sh
There was a problem hiding this comment.
Actually, consider adding it to /tool/travis_ios_script.sh as well.
There was a problem hiding this comment.
ok, I think I did. I'm not sure I understood the travis_ios_script. Did you mean whether I had an XCTest for this?
There was a problem hiding this comment.
No, I just want it to build. We have some full integration tests for the add-to-app Android apps, but haven't had bandwidth to do anything for iOS.
| ); | ||
| MTL_ENABLE_DEBUG_INFO = INCLUDE_SOURCE; | ||
| MTL_FAST_MATH = YES; | ||
| PRODUCT_BUNDLE_IDENTIFIER = com.example.iosAppClip.Clip; |
There was a problem hiding this comment.
Should we do the ${DEVELOPMENT_TEAM} trick for this project as well?
There was a problem hiding this comment.
And is this the right place, or should it be done in AppFrameworkInfo.plist? I don't actually know, and I need to update the other samples.
There was a problem hiding this comment.
I now realize it's probably weird to use your PR to work out my own research, but there it is. 😄
There was a problem hiding this comment.
Ah very good point. Done. PRODUCT_BUNDLE_IDENTIFIER is the thing to use, rather than CFBundleIdentifier.
|
Gentle ping |
|
LGTM. Adding it to the iOS test script felt like a thing to do because this app mostly exists to illustrate iOS configuration and iOS-related stuff. It's not completely necessary though. I'll have a think on it. Comments welcome in the meantime. |
1. Run `flutter run` on the project to pick up project automigrations. 2. Set min version to iOS 16 to take advantage of the 15MB size limit (see flutter/flutter#71098 (comment)) 3. Other updates needed to avoid Xcode errors and warnings. First introduced in #538 See also flutter/website#10145
2 participants
Don't know if I've crossed the threshold of acceptable scope 😅
I've made a sample to show using an App Clip target in iOS 14.
flutter/flutter#60029