Skip to content

[quick_actions] convert to pigeon #5159

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
auto-submit merged 13 commits into from
Oct 20, 2023
Merged

[quick_actions] convert to pigeon #5159

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
409cd34
swift
tarrinneal Oct 18, 2023
cbbd121
doc
tarrinneal Oct 18, 2023
9eaaaf2
Merge branch 'main' of github.com:flutter/packages into QA-pigeon
tarrinneal Oct 19, 2023
f23b103
Merge branch 'main' of github.com:flutter/packages into QA-pigeon
tarrinneal Oct 19, 2023
01599f2
nits and test fixes
tarrinneal Oct 20, 2023
975dcb2
Merge branch 'main' of github.com:flutter/packages into QA-pigeon
tarrinneal Oct 20, 2023
2ca3980
nits
tarrinneal Oct 20, 2023
110a311
delete file
tarrinneal Oct 20, 2023
20796bb
remove parser class
tarrinneal Oct 20, 2023
e2dc2b7
method names
tarrinneal Oct 20, 2023
bfe3748
non static
tarrinneal Oct 20, 2023
f8bf4b2
message
tarrinneal Oct 20, 2023
249d3f8
selfies
tarrinneal Oct 20, 2023
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
Prev Previous commit
Next Next commit
remove parser class
  • Loading branch information
@tarrinneal
tarrinneal committed Oct 20, 2023
commit 20796bb00899621d3efaf5fabddca5b87d64ef28
6 changes: 2 additions & 4 deletions ...ck_actions/quick_actions_ios/example/ios/RunnerTests/DefaultShortcutItemParserTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@ class DefaultShortcutItemParserTests: XCTestCase {
icon: UIApplicationShortcutIcon(templateImageName: "search_the_thing.png"),
userInfo: nil)

let parser = DefaultShortcutItemParser()
XCTAssertEqual(parser.parseShortcutItems([rawItem]), [expectedItem])
XCTAssertEqual(QuickActionsPlugin.parseShortcutItems([rawItem]), [expectedItem])
}

func testParseShortcutItems_noIcon() {
Expand All @@ -40,7 +39,6 @@ class DefaultShortcutItemParserTests: XCTestCase {
icon: nil,
userInfo: nil)

let parser = DefaultShortcutItemParser()
XCTAssertEqual(parser.parseShortcutItems([rawItem]), [expectedItem])
XCTAssertEqual(QuickActionsPlugin.parseShortcutItems([rawItem]), [expectedItem])
}
}
47 changes: 31 additions & 16 deletions packages/quick_actions/quick_actions_ios/ios/Classes/QuickActionsPlugin.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,17 @@ public final class QuickActionsPlugin: NSObject, FlutterPlugin, IOSQuickActionsA

public static func register(with registrar: FlutterPluginRegistrar) {
let messenger = registrar.messenger()
let instance = QuickActionsPlugin(messenger: messenger)
let flutterApi = IOSQuickActionsFlutterApi(binaryMessenger: messenger)
let instance = QuickActionsPlugin(flutterApi: flutterApi)
IOSQuickActionsApiSetup.setUp(binaryMessenger: messenger, api: instance)
registrar.addApplicationDelegate(instance)
}

private let shortcutItemProvider: ShortcutItemProviding
private let shortcutItemParser: ShortcutItemParser = DefaultShortcutItemParser()
private let flutterApi: IOSQuickActionsFlutterApiProtocol
/// The type of the shortcut item selected when launching the app.
private var launchingShortcutType: String? = nil

// This init is meant for unit testing only.
init(
flutterApi: IOSQuickActionsFlutterApiProtocol,
shortcutItemProvider: ShortcutItemProviding = UIApplication.shared
Expand All @@ -28,27 +27,19 @@ public final class QuickActionsPlugin: NSObject, FlutterPlugin, IOSQuickActionsA
self.shortcutItemProvider = shortcutItemProvider
}

convenience init(
messenger: FlutterBinaryMessenger,
shortcutItemProvider: ShortcutItemProviding = UIApplication.shared
) {
let flutterApi = IOSQuickActionsFlutterApi(binaryMessenger: messenger)
self.init(flutterApi: flutterApi, shortcutItemProvider: shortcutItemProvider)
}

func setShortcutItems(itemsList: [ShortcutItemMessage]) {
shortcutItemProvider.shortcutItems = shortcutItemParser.parseShortcutItems(itemsList)
self.shortcutItemProvider.shortcutItems = QuickActionsPlugin.parseShortcutItems(itemsList)
}

func clearShortcutItems() {
shortcutItemProvider.shortcutItems = []
self.shortcutItemProvider.shortcutItems = []
}

public func application(
_ application: UIApplication, performActionFor shortcutItem: UIApplicationShortcutItem,
completionHandler: @escaping (Bool) -> Void
) -> Bool {
handleShortcut(shortcutItem.type)
self.handleShortcut(shortcutItem.type)
return true
}

Expand All @@ -75,14 +66,38 @@ public final class QuickActionsPlugin: NSObject, FlutterPlugin, IOSQuickActionsA

public func applicationDidBecomeActive(_ application: UIApplication) {
if let shortcutType = launchingShortcutType {
handleShortcut(shortcutType)
self.handleShortcut(shortcutType)
self.launchingShortcutType = nil
}
}

func handleShortcut(_ shortcut: String) {
flutterApi.launchAction(action: shortcut) { _ in
self.flutterApi.launchAction(action: shortcut) { _ in
// noop
}
}

static func parseShortcutItems(_ items: [ShortcutItemMessage]) -> [UIApplicationShortcutItem] {
Copy link
Contributor

@hellohuanlin hellohuanlin Oct 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

this doesn't seem to belong to a static member of QuickActionsPlugin. I suggest 2 alternatives -

  1. make them private non-static members of QuickActionsPlugin (so they become implementation details QuickActionsPlugin).

  2. A more swifty way I would do is to create class extension of either UIApplicationShortcutItem or ShortcutItemMessage, outside of QuickActionsPlugin

Copy link
Contributor Author

@tarrinneal tarrinneal Oct 20, 2023

Choose a reason for hiding this comment

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

Sounds good, thanks for the feedback!

Copy link
Contributor

@hellohuanlin hellohuanlin Oct 20, 2023

Choose a reason for hiding this comment

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

Oh also avoid the naming "parse" since it's not parsing anymore. Something like:

extension ShortcutItemMessage {
  var item: UIApplicationShortcutItem {
    return ...
  }
}

Copy link
Contributor Author

@tarrinneal tarrinneal Oct 20, 2023

Choose a reason for hiding this comment

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

I don't think we need the tests that I was using this for any longer, since this is now an implementation detail and my understanding is that the swifty thing to do is consider the converter tested, since it is used in code that is already tested.

Copy link
Contributor

@hellohuanlin hellohuanlin Oct 20, 2023

Choose a reason for hiding this comment

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

Yes if it's implementation detail, it can only be tested via non-private members. You can delete those tests if it's already covered elsewhere

return items.compactMap { deserializeShortcutItem(with: $0) }
}

static private func deserializeShortcutItem(with serialized: ShortcutItemMessage)
-> UIApplicationShortcutItem?
{

let type = serialized.type
let localizedTitle = serialized.localizedTitle

let icon = (serialized.icon).map {
UIApplicationShortcutIcon(templateImageName: $0)
}

// type and localizedTitle are required.
return UIApplicationShortcutItem(
type: type,
localizedTitle: localizedTitle,
localizedSubtitle: nil,
icon: icon,
userInfo: nil)
}
}
44 changes: 0 additions & 44 deletions packages/quick_actions/quick_actions_ios/ios/Classes/ShortcutItemParser.swift
View file
Open in desktop

This file was deleted.