Add windows implementation to platform_view example #109715
Conversation
yaakovschectman
added
a: tests
flutter-dashboard
bot
added
d: examples
examples/platform_view/lib/main.dart
Outdated
| return const Text('Continue in iOS view'); | ||
| } else if (Platform.isWindows) { | ||
| return Text('Cotninue in Windows view'); | ||
| return const Text('Cotninue in Windows view'); |
| result->Success(flutter::EncodableValue(counter)); | ||
| } | ||
| ); | ||
| } |
There was a problem hiding this comment.
Give clang-format a run over this file. Line 22 is getting pretty long :)
cbracken
left a comment
cbracken
left a comment
There was a problem hiding this comment.
lgtm modulo a couple nits :)
examples/platform_view/lib/main.dart
Outdated
| }); | ||
| } | ||
|
|
||
| Text _getButtonText() { |
There was a problem hiding this comment.
nit: since this function has no parameter list, you could make it a static getter.
static Widget get buttonText {
// ... code here
}
you can choose to ignore this :)
examples/platform_view/lib/main.dart
Outdated
| } | ||
|
|
||
| Text _getButtonText() { | ||
| if (Platform.isAndroid) { |
There was a problem hiding this comment.
nit: use TargetPlatform and a switch case so that we know what is unimplemented
import 'package:flutter/foundation.dart';
// your code here...
switch(defaultTargetPlatform) {
case TargetPlatform.android:
return const Text('Continue in Android view');
case TargetPlatform.iOS:
return const Text('Continue in iOS view');
case TargetPlatform.windows:
return const Text('Continue in Windows view');
case TargetPlatform.fuchsia:
case TargetPlatform.macOS:
case TargetPlatform.linux:
throw UnimplementedError();
}You can choose to ignore this.
There was a problem hiding this comment.
Thanks for spotting this! I was going to make the same comment then ended up going "I could have sworn this was an enum at some point, but apparently not!". There is a small difference between the two -- apps can fake out the platform they're running in via the enum, but not via these checks. In this case that's fine :)
Years ago we did a blast through as much of the code as we could to kill if-else blocks on enums and replace with switch since our linter enforces that all cases are covered but looks like we didn't check the examples!
| #include <flutter/standard_method_codec.h> | ||
|
|
||
| #include <cstdint> | ||
| #include <iostream> |
There was a problem hiding this comment.
Is this used? Perhaps double-check that the other includes are also all used.
4 participants
Previously, the platform_view example project was implemented only for iOS and Android. As part of issue #70027 , a Windows implementation was added in order to add a startup test for this project. This project utilizes platform-dependent views for iOS and Android that are displayed when a button is pressed. While the startup test for Windows passed as-is already, nothing would happen when the button is pressed on Windows. This PR implements a message box that displays when the button is pressed.
Addresses issue #109683
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.