Plans: Plan Details #4782
Plans: Plan Details #4782
Conversation
* Added Purchase button * Removed use of ImmuTableViewHandler and added data source methods to PlanDetailViewController * Added alternating row colors for table rows.
| } | ||
|
|
||
| func ==(lhs: PlanFeature, rhs: PlanFeature) -> Bool { | ||
| return lhs.title == rhs.title |
There was a problem hiding this comment.
I think that this is not correct, even if it works for this case. I believe .Support("3GB") shouldn't be equal to .Support("13GB"), even when they'll share the same title.
You can compare values with pattern matching, although I think you'll have to cover every case.
There was a problem hiding this comment.
Hmm... the reason I added Equatable was simply so that I could find a plan's matching feature compared to the list of all features, for use here: https://github.com/wordpress-mobile/WordPress-iOS/blob/feature/plan-details/WordPress/Classes/ViewRelated/Plans/PlanDetailViewController.swift#L80
This wouldn't work if it also checked on the associated values etc too. I'm happy to restructure the features and do this differently if we can think of a better way though!
|
The 'WEB ONLY' label is slightly misaligned with respect to the feature title:
|
| layer.cornerRadius = cornerRadius | ||
| layer.borderWidth = borderWidth | ||
| layer.borderColor = tintColor.CGColor | ||
| layer.borderColor = tintColor.CGColor |
There was a problem hiding this comment.
⌘-C, ⌘-V... Thanks for spotting this!
|
I've fixed the WEB ONLY alignment, removed the duplicate line, and replaced the custom cells in @koke Would you mind checking over these changes please? |
Fixed unit tests by using a mock presenter instead of UIViewController
…resent Fix plans presentation
…ground. * PlanDetailViewController is now a standard VC, not a table view controller. * The header is now a subview of the VC, not a header of the table view.
…rdPress-iOS into feature/plan-details
| @@ -0,0 +1,51 @@ | |||
| import UIKit | |||
| import MRProgress | |||
There was a problem hiding this comment.
I just noticed this. It's not used, right?
There was a problem hiding this comment.
Good spot. No, it's not currently. I was experimenting with using it to display a loading state for the button when a purchase is being made (like the App Store). Doesn't belong on this branch!
|
|
Adds a plan details view, as part of #4745. This is currently just the detail view itself, with no comparison / plan switching functionality. It shows the details for the plan selected from
PlanListViewController.Known issues
– As with #4772, prices are hardcoded – this will be dealt with as part of #4746. The purchase button is also currently shown for the Free plan.
– The current active plan isn't highlighted yet. This will come with the comparison / switcher update.
– The data is currently hardcoded into the app. It would be nice as a future improvement to fetch this from our API instead to keep up to date.
– A few design (colour, mainly) tweaks may be required. iPad is currently not dealt with, as that'll be handled by the switcher view controller that wraps this one.
Needs Review: @koke