-
Notifications
You must be signed in to change notification settings - Fork 6k
hasStrings Mac #20531
hasStrings Mac #20531
Changes from 8 commits
bb0f4be
97acb46
660092e
ffc7af4
98adfc9
59ad159
084c926
1a22b66
67f0173
6177e74
24d28c8
51280f2
a15798e
878aa78
c2bca18
54301f3
70286fc
6c778fe
b68a594
25385b5
45b81b4
2d0728d
3c9ec6e
a52595d
d14a117
3490d8a
e1566ac
491a585
8c2d240
06a6034
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -173,6 +173,14 @@ - (NSDictionary*)getClipboardData:(NSString*)format; | |
| */ | ||
| - (void)setClipboardData:(NSDictionary*)data; | ||
|
|
||
| /** | ||
| * Returns true iff the clipboard contains nonempty string data. | ||
| * | ||
| * See also: | ||
| * * https://developer.apple.com/documentation/uikit/uipasteboard/1829416-hasstrings | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are you referencing UIKit documentation in the macOS embedding?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There seems to be no equivalent hasStrings method for Mac. I'll add some details in the comment. |
||
| */ | ||
| - (NSDictionary*)clipboardHasStrings; | ||
|
|
||
| @end | ||
|
|
||
| #pragma mark - FlutterViewController implementation. | ||
|
|
@@ -505,6 +513,8 @@ - (void)handleMethodCall:(FlutterMethodCall*)call result:(FlutterResult)result { | |
| } else if ([call.method isEqualToString:@"Clipboard.setData"]) { | ||
| [self setClipboardData:call.arguments]; | ||
| result(nil); | ||
| } else if ([call.method isEqualToString:@"Clipboard.hasStrings"]) { | ||
| result([self clipboardHasStrings]); | ||
| } else { | ||
| result(FlutterMethodNotImplemented); | ||
| } | ||
|
|
@@ -528,12 +538,19 @@ - (NSDictionary*)getClipboardData:(NSString*)format { | |
| - (void)setClipboardData:(NSDictionary*)data { | ||
| NSPasteboard* pasteboard = [NSPasteboard generalPasteboard]; | ||
| NSString* text = data[@"text"]; | ||
| [pasteboard clearContents]; | ||
| if (text && ![text isEqual:[NSNull null]]) { | ||
| [pasteboard clearContents]; | ||
| [pasteboard setString:text forType:NSPasteboardTypeString]; | ||
| } | ||
| } | ||
|
|
||
| - (NSDictionary*)clipboardHasStrings { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: It's a bit odd having a method called
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's a good call, the docs I wrote made this even more confusing actually. |
||
| NSDictionary* data = [self getClipboardData:[NSString stringWithFormat:@"%s", kTextPlainFormat]]; | ||
| NSString* string = data[@"text"]; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looking at this more closely this time: going through a method whose input and output are explicitly designed for the Flutter method channel contract seems like a very convoluted way to check this. Why not just
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point, done. |
||
| BOOL hasStrings = string.length > 0; | ||
| return @{@"value" : @(hasStrings)}; | ||
| } | ||
|
|
||
| #pragma mark - FlutterViewReshapeListener | ||
|
|
||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| // Copyright 2013 The Flutter Authors. All rights reserved. | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| #include "flutter/shell/platform/darwin/macos/framework/Headers/FlutterEngine.h" | ||
| #import "flutter/shell/platform/darwin/macos/framework/Headers/FlutterViewController.h" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the primary header for the test, so should be first in its own block. |
||
| #include "flutter/shell/platform/darwin/macos/framework/Source/FlutterDartProject_Internal.h" | ||
| #include "flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h" | ||
| #include "flutter/testing/testing.h" | ||
|
|
||
| namespace flutter::testing { | ||
|
|
||
| TEST(FlutterViewControllerTest, MacOSTestTest) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this called MacOSTestTest? The name should describe what the test is covering.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, forgot to update that. |
||
| NSString* fixtures = @(testing::GetFixturesPath()); | ||
| FlutterDartProject* project = [[FlutterDartProject alloc] | ||
| initWithAssetsPath:fixtures | ||
| ICUDataPath:[fixtures stringByAppendingString:@"/icudtl.dat"]]; | ||
| FlutterViewController* viewController = [[FlutterViewController alloc] initWithProject:project]; | ||
|
|
||
| // First call setClipboardData to put a string on the pasteboard. | ||
| __block bool calledSet = false; | ||
| FlutterResult resultSet = ^(id result) { | ||
| calledSet = true; | ||
| }; | ||
| FlutterMethodCall* methodCallSet = | ||
| [FlutterMethodCall methodCallWithMethodName:@"Clipboard.setData" | ||
| arguments:@{@"text" : @"some string"}]; | ||
| [viewController handleMethodCall:methodCallSet result:resultSet]; | ||
| ASSERT_TRUE(calledSet); | ||
|
|
||
| // Call hasStrings and expect it to be true. | ||
| __block bool called = false; | ||
| __block bool value; | ||
| FlutterResult result = ^(id result) { | ||
| called = true; | ||
| NSNumber* valueNumber = [result valueForKey:@"value"]; | ||
| value = [valueNumber boolValue]; | ||
| }; | ||
| FlutterMethodCall* methodCall = | ||
| [FlutterMethodCall methodCallWithMethodName:@"Clipboard.hasStrings" arguments:nil]; | ||
| [viewController handleMethodCall:methodCall result:result]; | ||
| ASSERT_TRUE(called); | ||
| ASSERT_TRUE(value); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suspect the unit tests on the bots are running in a bootstrap context that doesn't have access to the pasteboard server. If that's the case, perhaps the tests should inject a mock
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any idea how to include OCMock in the Mac tests? I tried including it in this test file and adding it to Build.gn in various places, but I keep getting compiler errors.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The same way it's done for iOS? Without knowing what you tried and what exactly didn't work, it's hard to advise further. |
||
|
|
||
| // Now call setData again to clear the pasteboard. | ||
| __block bool calledSetClear = false; | ||
| FlutterResult resultSetClear = ^(id result) { | ||
| calledSetClear = true; | ||
| }; | ||
| FlutterMethodCall* methodCallSetClear = | ||
| [FlutterMethodCall methodCallWithMethodName:@"Clipboard.setData" | ||
| arguments:@{@"text" : [NSNull null]}]; | ||
| [viewController handleMethodCall:methodCallSetClear result:resultSetClear]; | ||
| ASSERT_TRUE(calledSetClear); | ||
|
|
||
| // Call hasStrings and expect it to be false. | ||
| __block bool calledAfterClear = false; | ||
| __block bool valueAfterClear; | ||
| FlutterResult resultAfterClear = ^(id result) { | ||
| calledAfterClear = true; | ||
| NSNumber* valueNumber = [result valueForKey:@"value"]; | ||
| valueAfterClear = [valueNumber boolValue]; | ||
| }; | ||
| FlutterMethodCall* methodCallAfterClear = | ||
| [FlutterMethodCall methodCallWithMethodName:@"Clipboard.hasStrings" arguments:nil]; | ||
| [viewController handleMethodCall:methodCallAfterClear result:resultAfterClear]; | ||
| ASSERT_TRUE(calledAfterClear); | ||
| ASSERT_FALSE(valueAfterClear); | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test is doing a lot of different things; please make individual tests that are more targeted. You're already using a fake pasteboard, so there's no need to do the entire cycle in a single test.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good call, I split it into two and it's a lot cleaner. |
||
|
|
||
| } // flutter::testing | ||
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.
Why are you making this public?
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.
Good catch, I thought this was necessary to call it from the test, but it seems to work without it. I'll remove it.