Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

hasStrings Mac #20531

Merged
justinmc merged 30 commits into from
Sep 15, 2020
Merged

hasStrings Mac #20531

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
bb0f4be
hasStrings on mac
justinmc Aug 14, 2020
97acb46
WIP test
justinmc Aug 15, 2020
660092e
Clean up and remove test since unit testing on mac not set up?
justinmc Aug 27, 2020
ffc7af4
Test hasStrings
justinmc Aug 31, 2020
98adfc9
Test hasStrings after clearing the clipboard as well
justinmc Aug 31, 2020
59ad159
Run Mac unit tests in run_tests.py
justinmc Aug 31, 2020
084c926
Formatting
justinmc Aug 31, 2020
1a22b66
Build.gn formatting
justinmc Aug 31, 2020
67f0173
Test name
justinmc Sep 1, 2020
6177e74
WIP trying to get ocmock to work in Mac
justinmc Sep 2, 2020
24d28c8
Use new ocmock target coming in buildroot PR
justinmc Sep 3, 2020
51280f2
Mocked pasteboard working
justinmc Sep 3, 2020
a15798e
handleMethodCall removed from header and pasteboard moved to property…
justinmc Sep 3, 2020
878aa78
Docs clarification
justinmc Sep 3, 2020
c2bca18
clipboardHasStrings returns a bool, caller handles dictionary
justinmc Sep 3, 2020
54301f3
Split tests and some cleanup
justinmc Sep 3, 2020
70286fc
Deduplicate mocking code
justinmc Sep 3, 2020
6c778fe
Comment cleanup
justinmc Sep 3, 2020
b68a594
Analyzer fixes
justinmc Sep 3, 2020
25385b5
Merge branch 'master' into has-strings-mac
justinmc Sep 3, 2020
45b81b4
Reference new buildroot hash after my change to ocmock there
justinmc Sep 4, 2020
2d0728d
Merge branch 'master' into has-strings-mac
justinmc Sep 8, 2020
3c9ec6e
_pasteboard -> pasteboard
justinmc Sep 9, 2020
a52595d
Mock pasteboard value directly instead of mocking setString
justinmc Sep 9, 2020
d14a117
Don't run the mac tests with run_test.py, because they fail in AOT mode
justinmc Sep 9, 2020
3490d8a
Merge branch 'master' into has-strings-mac
justinmc Sep 9, 2020
e1566ac
pasteboard in the private header
justinmc Sep 10, 2020
491a585
Merge branch 'master' into has-strings-mac
justinmc Sep 11, 2020
8c2d240
Merge branch 'master' into has-strings-mac
justinmc Sep 11, 2020
06a6034
Use ocmock now that it has been made to work with mac and ocmock_src …
justinmc Sep 12, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ci/licenses_golden/licenses_flutter
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1027,6 +1027,7 @@ FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterTextI
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterView.h
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterView.mm
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTest.mm
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h
FILE: ../../../flutter/shell/platform/darwin/macos/framework/Source/fixtures/flutter_desktop_test.dart
FILE: ../../../flutter/shell/platform/darwin/macos/framework/module.modulemap
Expand Down
5 changes: 4 additions & 1 deletion shell/platform/darwin/macos/BUILD.gn
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,10 @@ test_fixtures("flutter_desktop_darwin_fixtures") {
executable("flutter_desktop_darwin_unittests") {
testonly = true

sources = [ "framework/Source/FlutterEngineUnittests.mm" ]
sources = [
"framework/Source/FlutterEngineUnittests.mm",
"framework/Source/FlutterViewControllerTest.mm"
]

cflags_objcc = [ "-fobjc-arc" ]

Expand Down
17 changes: 17 additions & 0 deletions shell/platform/darwin/macos/framework/Source/FlutterViewController.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g Sep 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you referencing UIKit documentation in the macOS embedding?

Copy link
Contributor Author

@justinmc justinmc Sep 3, 2020

Choose a reason for hiding this comment

The 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.
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -534,6 +544,13 @@ - (void)setClipboardData:(NSDictionary*)data {
}
}

- (NSDictionary*)clipboardHasStrings {
Copy link
Member

@gaaclarke gaaclarke Sep 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: It's a bit odd having a method called clipboardHasStrings return something other than a bool. It is probably better to return bool and wrap it into a dictionary at the call site, imo.

Copy link
Contributor Author

@justinmc justinmc Sep 3, 2020

Choose a reason for hiding this comment

The 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"];
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g Sep 8, 2020

Choose a reason for hiding this comment

The 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 return [self.pasteboard stringForType:NSPasteboardTypeString]].length > 0;?

Copy link
Contributor Author

@justinmc justinmc Sep 9, 2020

Choose a reason for hiding this comment

The 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

/**
Expand Down