Implement SDL-0085 SubMenu Icon #997
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
joeljfischer
changed the title
NicoleYarroch
suggested changes
Jun 27, 2018
# Conflicts: # SmartDeviceLink_Example/MenuManager.m # SmartDeviceLink_Example/MenuManager.swift
| submenu.position = @(position); | ||
|
|
||
| return submenu; | ||
| - (SDLAddSubMenu *)sdl_subMenuCommandForMenuCell:(SDLMenuCell *)cell withArtwork:(BOOL)shouldHaveArtwork position:(UInt16)position { |
There was a problem hiding this comment.
If sending two submenus, one with artwork and one without, the manager will add an image without a name to the the AddSubMenu RPC. The command then fails because the image is missing a name.
The following method should also check if cell.icon.name is nil and not just check if shouldHaveArtwork is true.
SDLImage *icon = shouldHaveArtwork ? [[SDLImage alloc] initWithName:cell.icon.name] : nil;
* Fix not using templates
|
|
||
| return SDLMenuCell(title: ACSubmenuMenuName, subCells: submenuItems) | ||
| return SDLMenuCell(title: ACSubmenuMenuName, icon: SDLArtwork(image: #imageLiteral(resourceName: "choice_set"), persistent: true, as: .PNG), subCells: submenuItems) |
There was a problem hiding this comment.
The artwork needs to be templated.
| @@ -119,7 +119,7 @@ private extension MenuManager { | |||
| /// - Returns: A SDLMenuCell object | |||
| class func menuCellWithSubmenu(with manager: SDLManager) -> SDLMenuCell { | |||
| var submenuItems = [SDLMenuCell]() | |||
| for i in 0..<75 { | |||
| for i in 0 ..< 10 { | |||
| let submenuTitle = "Submenu Item \(i)" | |||
| submenuItems.append(SDLMenuCell(title: submenuTitle, icon: SDLArtwork(image: UIImage(named: MenuBWIconImageName)!, persistent: true, as: .PNG), voiceCommands: [submenuTitle, "Item \(i)", "\(i)"], handler: { (triggerSource) in | |||
There was a problem hiding this comment.
The artwork needs to be templated
NicoleYarroch
suggested changes
Jun 29, 2018
| @@ -63,7 +63,8 @@ private extension MenuManager { | |||
| VehicleDataManager.getAllVehicleData(with: manager, triggerSource: triggerSource, vehicleDataType: submenuName) | |||
| }) | |||
| } | |||
| return SDLMenuCell(title: ACGetAllVehicleDataMenuName, subCells: submenuItems) | |||
|
|
|||
| return SDLMenuCell(title: ACGetAllVehicleDataMenuName, icon: UIImage(named: CarBWIconImageName)!.withRenderingMode(.alwaysTemplate) subCells: submenuItems) | |||
| NSArray *submenu = [[mockConnectionManager.receivedRequests copy] filteredArrayUsingPredicate:addSubmenuPredicate]; | ||
| SDLAddSubMenu *sentSubmenu = submenu.firstObject; | ||
|
|
||
| expect(add).to(haveCount(2)); |
There was a problem hiding this comment.
There are a few failed test cases in this spec
NicoleYarroch
suggested changes
Jul 10, 2018
| @@ -47,18 +47,28 @@ @interface SDLMenuManager() | |||
|
|
|||
| QuickSpecBegin(SDLMenuManagerSpec) | |||
|
|
|||
| describe(@"menu manager", ^{ | |||
| fdescribe(@"menu manager", ^{ | |||
There was a problem hiding this comment.
Remove the f from fdescribe
NicoleYarroch
suggested changes
Jul 11, 2018
NicoleYarroch
approved these changes
Jul 13, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #719
This PR is ready for review.
Risk
This PR makes minor API changes.
Testing Plan
Unit tests are added, tested against Ford Sync Gen 3 and Core v5.0.0 in-dev.
Summary
Submenu commands / menu cells with subcells may now contain artwork.
Changelog
Enhancements
CLA