-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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.
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
[Site Name] Adjusts design picker buttons for treatment variation #18397
Changes from all commits
d9fbe30
4c0a984
082306a
7a4701d
9ecd2b8
e2f575c
babc542
7ad2a9d
00accc5
fc7ddd3
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 |
---|---|---|
|
@@ -27,6 +27,7 @@ class SiteDesignSection: CategorySection { | |
|
||
class SiteDesignContentCollectionViewController: FilterableCategoriesViewController, UIPopoverPresentationControllerDelegate { | ||
typealias TemplateGroup = SiteDesignRequest.TemplateGroup | ||
private let createsSite: Bool | ||
private let templateGroups: [TemplateGroup] = [.stable, .singlePage] | ||
|
||
let completion: SiteDesignStep.SiteDesignSelection | ||
|
@@ -55,15 +56,16 @@ class SiteDesignContentCollectionViewController: FilterableCategoriesViewControl | |
return sections[sectionIndex].designs[position] | ||
} | ||
|
||
init(_ completion: @escaping SiteDesignStep.SiteDesignSelection) { | ||
init(createsSite: Bool, _ completion: @escaping SiteDesignStep.SiteDesignSelection) { | ||
self.completion = completion | ||
self.createsSite = createsSite | ||
|
||
super.init( | ||
analyticsLocation: "site_creation", | ||
mainTitle: NSLocalizedString("Choose a design", comment: "Title for the screen to pick a design and homepage for a site."), | ||
prompt: NSLocalizedString("Pick your favorite homepage layout. You can edit and customize it later.", comment: "Prompt for the screen to pick a design and homepage for a site."), | ||
primaryActionTitle: NSLocalizedString("Choose", comment: "Title for the button to progress with the selected site homepage design"), | ||
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. similar comments here: the comment should have a period, and since this file has quite a few hardcoded strings, wondering if we can group them? 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. As above handled with babc542. Thank you for the feedback 🙇 |
||
secondaryActionTitle: NSLocalizedString("Preview", comment: "Title for button to preview a selected homepage design") | ||
mainTitle: TextContent.mainTitle, | ||
prompt: TextContent.subtitle, | ||
primaryActionTitle: createsSite ? TextContent.createSiteButton : TextContent.chooseButton, | ||
secondaryActionTitle: TextContent.previewButton | ||
) | ||
} | ||
|
||
|
@@ -73,7 +75,7 @@ class SiteDesignContentCollectionViewController: FilterableCategoriesViewControl | |
|
||
override func viewDidLoad() { | ||
super.viewDidLoad() | ||
navigationItem.backButtonTitle = NSLocalizedString("Design", comment: "Shortened version of the main title to be used in back navigation") | ||
navigationItem.backButtonTitle = TextContent.backButtonTitle | ||
fetchSiteDesigns() | ||
configureCloseButton() | ||
configureSkipButton() | ||
|
@@ -100,7 +102,7 @@ class SiteDesignContentCollectionViewController: FilterableCategoriesViewControl | |
} | ||
|
||
private func configureSkipButton() { | ||
let skip = UIBarButtonItem(title: NSLocalizedString("Skip", comment: "Continue without making a selection"), style: .done, target: self, action: #selector(skipButtonTapped)) | ||
let skip = UIBarButtonItem(title: TextContent.skipButtonTitle, style: .done, target: self, action: #selector(skipButtonTapped)) | ||
navigationItem.rightBarButtonItem = skip | ||
} | ||
|
||
|
@@ -115,7 +117,7 @@ class SiteDesignContentCollectionViewController: FilterableCategoriesViewControl | |
return | ||
} | ||
|
||
navigationItem.leftBarButtonItem = UIBarButtonItem(title: NSLocalizedString("Cancel", comment: "Cancel site creation"), style: .done, target: self, action: #selector(closeButtonTapped)) | ||
navigationItem.leftBarButtonItem = UIBarButtonItem(title: TextContent.cancelButtonTitle, style: .done, target: self, action: #selector(closeButtonTapped)) | ||
} | ||
|
||
@objc func skipButtonTapped(_ sender: Any) { | ||
|
@@ -151,7 +153,7 @@ class SiteDesignContentCollectionViewController: FilterableCategoriesViewControl | |
override func secondaryActionSelected(_ sender: Any) { | ||
guard let design = selectedDesign else { return } | ||
|
||
let previewVC = SiteDesignPreviewViewController(siteDesign: design, selectedPreviewDevice: selectedPreviewDevice, onDismissWithDeviceSelected: { [weak self] device in | ||
let previewVC = SiteDesignPreviewViewController(siteDesign: design, selectedPreviewDevice: selectedPreviewDevice, createsSite: createsSite, onDismissWithDeviceSelected: { [weak self] device in | ||
self?.selectedPreviewDevice = device | ||
}, completion: completion) | ||
|
||
|
@@ -162,10 +164,23 @@ class SiteDesignContentCollectionViewController: FilterableCategoriesViewControl | |
|
||
private func handleError(_ error: Error) { | ||
SiteCreationAnalyticsHelper.trackError(error) | ||
let titleText = NSLocalizedString("Unable to load this content right now.", comment: "Informing the user that a network request failed becuase the device wasn't able to establish a network connection.") | ||
let subtitleText = NSLocalizedString("Check your network connection and try again.", comment: "Default subtitle for no-results when there is no connection.") | ||
let titleText = TextContent.errorTitle | ||
let subtitleText = TextContent.errorSubtitle | ||
displayNoResultsController(title: titleText, subtitle: subtitleText, resultsDelegate: self) | ||
} | ||
|
||
private enum TextContent { | ||
static let mainTitle = NSLocalizedString("Choose a design", comment: "Title for the screen to pick a design and homepage for a site.") | ||
static let subtitle = NSLocalizedString("Pick your favorite homepage layout. You can edit and customize it later.", comment: "Prompt for the screen to pick a design and homepage for a site.") | ||
static let createSiteButton = NSLocalizedString("Create Site", comment: "Title for the button to progress with creating the site with the selected design.") | ||
static let chooseButton = NSLocalizedString("Choose", comment: "Title for the button to progress with the selected site homepage design.") | ||
static let previewButton = NSLocalizedString("Preview", comment: "Title for button to preview a selected homepage design.") | ||
static let backButtonTitle = NSLocalizedString("Design", comment: "Shortened version of the main title to be used in back navigation.") | ||
static let skipButtonTitle = NSLocalizedString("Skip", comment: "Continue without making a selection.") | ||
static let cancelButtonTitle = NSLocalizedString("Cancel", comment: "Cancel site creation.") | ||
static let errorTitle = NSLocalizedString("Unable to load this content right now.", comment: "Informing the user that a network request failed becuase the device wasn't able to establish a network connection.") | ||
static let errorSubtitle = NSLocalizedString("Check your network connection and try again.", comment: "Default subtitle for no-results when there is no connection.") | ||
} | ||
} | ||
|
||
// MARK: - NoResultsViewControllerDelegate | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,76 @@ | ||
|
||
import XCTest | ||
@testable import WordPress | ||
|
||
class SiteDesignTests: XCTestCase { | ||
private var remoteDesign: RemoteSiteDesign { | ||
let siteDesignPayload = "{\"slug\":\"alves\",\"title\":\"Alves\",\"segment_id\":1,\"categories\":[{\"slug\":\"business\",\"title\":\"Business\",\"description\":\"Business\",\"emoji\":\"💼\"}],\"demo_url\":\"https://public-api.wordpress.com/rest/v1/template/demo/alves/alvesstartermobile.wordpress.com/?language=en\",\"theme\":\"alves\",\"preview\":\"https://s0.wp.com/mshots/v1/public-api.wordpress.com/rest/v1/template/demo/alves/alvesstartermobile.wordpress.com/%3Flanguage%3Den?vpw=1200&vph=1600&w=800&h=1067\",\"preview_tablet\":\"https://s0.wp.com/mshots/v1/public-api.wordpress.com/rest/v1/template/demo/alves/alvesstartermobile.wordpress.com/%3Flanguage%3Den?vpw=800&vph=1066&w=800&h=1067\",\"preview_mobile\":\"https://s0.wp.com/mshots/v1/public-api.wordpress.com/rest/v1/template/demo/alves/alvesstartermobile.wordpress.com/%3Flanguage%3Den?vpw=400&vph=533&w=400&h=534\"}" | ||
return try! JSONDecoder().decode(RemoteSiteDesign.self, from: siteDesignPayload.data(using: .utf8)!) | ||
} | ||
|
||
func testSiteDesignPrimaryButtonTextNotLastStep() throws { | ||
|
||
// given | ||
let creator = SiteCreator() | ||
let siteDesignStep = SiteDesignStep(creator: creator, isLastStep: false) | ||
let expectedPrimaryTitle = "Choose" | ||
|
||
// when | ||
let siteDesignVC = try XCTUnwrap(siteDesignStep.content as? SiteDesignContentCollectionViewController) | ||
siteDesignVC.loadViewIfNeeded() | ||
siteDesignVC.viewDidLoad() | ||
|
||
// then | ||
let currentTitle = siteDesignVC.primaryActionButton.currentTitle | ||
XCTAssertEqual(expectedPrimaryTitle, currentTitle) | ||
} | ||
|
||
func testSiteDesignPrimaryButtonTextLastStep() throws { | ||
|
||
// given | ||
let creator = SiteCreator() | ||
let siteDesignStep = SiteDesignStep(creator: creator, isLastStep: true) | ||
let expectedPrimaryTitle = "Create Site" | ||
|
||
// when | ||
let siteDesignVC = try XCTUnwrap(siteDesignStep.content as? SiteDesignContentCollectionViewController) | ||
siteDesignVC.loadViewIfNeeded() | ||
siteDesignVC.viewDidLoad() | ||
|
||
// then | ||
let currentTitle = siteDesignVC.primaryActionButton.currentTitle | ||
XCTAssertEqual(expectedPrimaryTitle, currentTitle) | ||
} | ||
|
||
func testSiteDesignPreviewButtonTextNotLastStep() throws { | ||
|
||
// given | ||
let siteDesignPreviewVC = SiteDesignPreviewViewController( | ||
siteDesign: remoteDesign, selectedPreviewDevice: nil, createsSite: false, onDismissWithDeviceSelected: nil, completion: {design in }) | ||
let expectedPrimaryTitle = "Choose" | ||
|
||
// when | ||
siteDesignPreviewVC.loadViewIfNeeded() | ||
siteDesignPreviewVC.viewDidLoad() | ||
|
||
// then | ||
let currentTitle = siteDesignPreviewVC.primaryActionButton.currentTitle | ||
XCTAssertEqual(expectedPrimaryTitle, currentTitle) | ||
} | ||
|
||
func testSiteDesignPreviewButtonTextLastStep() throws { | ||
|
||
// given | ||
let siteDesignPreviewVC = SiteDesignPreviewViewController( | ||
siteDesign: remoteDesign, selectedPreviewDevice: nil, createsSite: true, onDismissWithDeviceSelected: nil, completion: {design in }) | ||
let expectedPrimaryTitle = "Create Site" | ||
|
||
// when | ||
siteDesignPreviewVC.loadViewIfNeeded() | ||
siteDesignPreviewVC.viewDidLoad() | ||
|
||
// then | ||
let currentTitle = siteDesignPreviewVC.primaryActionButton.currentTitle | ||
XCTAssertEqual(expectedPrimaryTitle, currentTitle) | ||
} | ||
} |
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.
Nice, good suggestion @Gio2018 and implementation @antonis. Makes things a lot cleaner!