-
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 Design Revamp] Main View - remove filtering and action buttons #18506
Merged
twstokes
merged 8 commits into
feature/site-design-revamp
from
task/site-design-remove-buttons-filtering
May 5, 2022
Merged
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
b707b9c
Remove UIPopoverPresentationControllerDelegate conformance.
twstokes 2240c7b
Add options to cells to disable decorations upon selection.
twstokes 8ccca1f
Remove action button tests now that we don't show action buttons.
twstokes 3870555
Subclass CollapsableHeaderViewController directly to remote filtering…
twstokes abed465
Add extension to Jetpack target.
twstokes d37f4e4
Make selectedPreviewDevice constant since we're no longer subclassing.
twstokes 8b0e649
Only toggle the checkmark for CollapsableHeaderCollectionViewCells an…
twstokes d17c426
Remove unnecessary test now that we're using a constant.
twstokes File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
7 changes: 7 additions & 0 deletions
7
...Press/Classes/ViewRelated/Site Creation/Design Selection/RemoteSiteDesign+Thumbnail.swift
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
import Foundation | ||
|
||
extension RemoteSiteDesign: Thumbnail { | ||
var urlDesktop: String? { screenshot } | ||
var urlTablet: String? { tabletScreenshot } | ||
var urlMobile: String? { mobileScreenshot} | ||
} |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,49 +1,32 @@ | ||
import UIKit | ||
import WordPressKit | ||
|
||
extension RemoteSiteDesign: Thumbnail { | ||
var urlDesktop: String? { screenshot } | ||
var urlTablet: String? { tabletScreenshot } | ||
var urlMobile: String? { mobileScreenshot} | ||
} | ||
|
||
class SiteDesignSection: CategorySection { | ||
var category: RemoteSiteDesignCategory | ||
var designs: [RemoteSiteDesign] | ||
|
||
var categorySlug: String { category.slug } | ||
var title: String { category.title } | ||
var emoji: String? { category.emoji } | ||
var description: String? { category.description } | ||
var thumbnails: [Thumbnail] { designs } | ||
var scrollOffset: CGPoint | ||
|
||
init(category: RemoteSiteDesignCategory, designs: [RemoteSiteDesign]) { | ||
self.category = category | ||
self.designs = designs | ||
self.scrollOffset = .zero | ||
} | ||
} | ||
|
||
class SiteDesignContentCollectionViewController: FilterableCategoriesViewController, UIPopoverPresentationControllerDelegate { | ||
class SiteDesignContentCollectionViewController: CollapsableHeaderViewController { | ||
typealias TemplateGroup = SiteDesignRequest.TemplateGroup | ||
typealias PreviewDevice = PreviewDeviceSelectionViewController.PreviewDevice | ||
|
||
private let createsSite: Bool | ||
private let templateGroups: [TemplateGroup] = [.stable, .singlePage] | ||
|
||
let completion: SiteDesignStep.SiteDesignSelection | ||
let restAPI = WordPressComRestApi.anonymousApi(userAgent: WPUserAgent.wordPress(), localeKey: WordPressComRestApi.LocaleKeyV2) | ||
var selectedIndexPath: IndexPath? = nil | ||
private let tableView: UITableView | ||
private let completion: SiteDesignStep.SiteDesignSelection | ||
private let restAPI = WordPressComRestApi.anonymousApi( | ||
userAgent: WPUserAgent.wordPress(), | ||
localeKey: WordPressComRestApi.LocaleKeyV2 | ||
) | ||
private var sections: [SiteDesignSection] = [] | ||
internal override var categorySections: [CategorySection] { get { sections }} | ||
private var isLoading: Bool = true { | ||
didSet { | ||
if isLoading { | ||
tableView.startGhostAnimation(style: GhostCellStyle.muriel) | ||
} else { | ||
tableView.stopGhostAnimation() | ||
} | ||
|
||
override var selectedPreviewDevice: PreviewDevice { | ||
get { .mobile } | ||
set { /* no op */ } | ||
tableView.reloadData() | ||
} | ||
} | ||
|
||
private lazy var previewViewSelectedPreviewDevice = PreviewDevice.default | ||
|
||
var siteDesigns = RemoteSiteDesigns() { | ||
private var previewViewSelectedPreviewDevice = PreviewDevice.default | ||
private var siteDesigns = RemoteSiteDesigns() { | ||
didSet { | ||
if oldValue.categories.count == 0 { | ||
scrollableView.setContentOffset(.zero, animated: false) | ||
|
@@ -52,26 +35,29 @@ class SiteDesignContentCollectionViewController: FilterableCategoriesViewControl | |
SiteDesignSection(category: category, designs: siteDesigns.designs.filter { design in design.categories.map({$0.slug}).contains(category.slug) | ||
}) | ||
} | ||
NSLog("sections: %@", String(describing: sections)) | ||
contentSizeWillChange() | ||
tableView.reloadData() | ||
} | ||
} | ||
|
||
var selectedDesign: RemoteSiteDesign? { | ||
guard let sectionIndex = selectedItem?.section, let position = selectedItem?.item else { return nil } | ||
return sections[sectionIndex].designs[position] | ||
var selectedPreviewDevice: PreviewDevice { | ||
get { .mobile } | ||
set { /* no op */ } | ||
} | ||
|
||
init(createsSite: Bool, _ completion: @escaping SiteDesignStep.SiteDesignSelection) { | ||
self.completion = completion | ||
self.createsSite = createsSite | ||
tableView = UITableView(frame: .zero, style: .plain) | ||
tableView.separatorStyle = .singleLine | ||
tableView.separatorInset = .zero | ||
tableView.showsVerticalScrollIndicator = false | ||
|
||
super.init( | ||
analyticsLocation: "site_creation", | ||
scrollableView: tableView, | ||
mainTitle: TextContent.mainTitle, | ||
primaryActionTitle: createsSite ? TextContent.createSiteButton : TextContent.chooseButton, | ||
secondaryActionTitle: TextContent.previewButton | ||
// the primary action button is never shown | ||
primaryActionTitle: "" | ||
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. This is the only part where |
||
) | ||
} | ||
|
||
|
@@ -81,6 +67,8 @@ class SiteDesignContentCollectionViewController: FilterableCategoriesViewControl | |
|
||
override func viewDidLoad() { | ||
super.viewDidLoad() | ||
tableView.register(CategorySectionTableViewCell.nib, forCellReuseIdentifier: CategorySectionTableViewCell.cellReuseIdentifier) | ||
tableView.dataSource = self | ||
navigationItem.backButtonTitle = TextContent.backButtonTitle | ||
fetchSiteDesigns() | ||
configureCloseButton() | ||
|
@@ -119,39 +107,18 @@ class SiteDesignContentCollectionViewController: FilterableCategoriesViewControl | |
navigationItem.leftBarButtonItem = UIBarButtonItem(title: TextContent.cancelButtonTitle, style: .done, target: self, action: #selector(closeButtonTapped)) | ||
} | ||
|
||
@objc func skipButtonTapped(_ sender: Any) { | ||
@objc | ||
private func closeButtonTapped(_ sender: Any) { | ||
dismiss(animated: true) | ||
} | ||
|
||
@objc | ||
private func skipButtonTapped(_ sender: Any) { | ||
presentedViewController?.dismiss(animated: true) | ||
SiteCreationAnalyticsHelper.trackSiteDesignSkipped() | ||
completion(nil) | ||
} | ||
|
||
override func primaryActionSelected(_ sender: Any) { | ||
guard let design = selectedDesign else { | ||
completion(nil) | ||
return | ||
} | ||
SiteCreationAnalyticsHelper.trackSiteDesignSelected(design) | ||
completion(design) | ||
} | ||
|
||
override func secondaryActionSelected(_ sender: Any) { | ||
guard let design = selectedDesign else { return } | ||
|
||
let previewVC = SiteDesignPreviewViewController( | ||
siteDesign: design, | ||
selectedPreviewDevice: previewViewSelectedPreviewDevice, | ||
createsSite: createsSite, | ||
onDismissWithDeviceSelected: { [weak self] device in | ||
self?.previewViewSelectedPreviewDevice = device | ||
}, | ||
completion: completion | ||
) | ||
|
||
let navController = GutenbergLightNavigationController(rootViewController: previewVC) | ||
navController.modalPresentationStyle = .pageSheet | ||
navigationController?.present(navController, animated: true) | ||
} | ||
|
||
private func handleError(_ error: Error) { | ||
SiteCreationAnalyticsHelper.trackError(error) | ||
let titleText = TextContent.errorTitle | ||
|
@@ -161,9 +128,6 @@ class SiteDesignContentCollectionViewController: FilterableCategoriesViewControl | |
|
||
private enum TextContent { | ||
static let mainTitle = NSLocalizedString("Choose a theme", comment: "Title for the screen to pick a theme 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.") | ||
|
@@ -173,8 +137,68 @@ class SiteDesignContentCollectionViewController: FilterableCategoriesViewControl | |
} | ||
|
||
// MARK: - NoResultsViewControllerDelegate | ||
|
||
extension SiteDesignContentCollectionViewController: NoResultsViewControllerDelegate { | ||
func actionButtonPressed() { | ||
fetchSiteDesigns() | ||
} | ||
} | ||
|
||
// MARK: - UITableViewDataSource | ||
|
||
extension SiteDesignContentCollectionViewController: UITableViewDataSource { | ||
|
||
func tableView(_ tableView: UITableView, estimatedHeightForRowAt indexPath: IndexPath) -> CGFloat { | ||
return CategorySectionTableViewCell.estimatedCellHeight | ||
} | ||
|
||
func tableView(_ tableView: UITableView, numberOfRowsInSection section: Int) -> Int { | ||
return isLoading ? 1 : (sections.count) | ||
} | ||
|
||
func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell { | ||
let cellReuseIdentifier = CategorySectionTableViewCell.cellReuseIdentifier | ||
guard let cell = tableView.dequeueReusableCell(withIdentifier: cellReuseIdentifier, for: indexPath) as? CategorySectionTableViewCell else { | ||
fatalError("Expected the cell with identifier \"\(cellReuseIdentifier)\" to be a \(CategorySectionTableViewCell.self). Please make sure the table view is registering the correct nib before loading the data") | ||
} | ||
cell.delegate = self | ||
cell.selectionStyle = UITableViewCell.SelectionStyle.none | ||
cell.section = isLoading ? nil : sections[indexPath.row] | ||
cell.isGhostCell = isLoading | ||
cell.selectionDecorationsHidden = true | ||
cell.layer.masksToBounds = false | ||
cell.clipsToBounds = false | ||
cell.collectionView.allowsSelection = !isLoading | ||
return cell | ||
} | ||
} | ||
|
||
// MARK: - CategorySectionTableViewCellDelegate | ||
|
||
extension SiteDesignContentCollectionViewController: CategorySectionTableViewCellDelegate { | ||
func didSelectItemAt(_ position: Int, forCell cell: CategorySectionTableViewCell, slug: String) { | ||
guard let sectionIndex = sections.firstIndex(where: { $0.categorySlug == slug }) else { return } | ||
let design = sections[sectionIndex].designs[position] | ||
|
||
let previewVC = SiteDesignPreviewViewController( | ||
siteDesign: design, | ||
selectedPreviewDevice: previewViewSelectedPreviewDevice, | ||
createsSite: createsSite, | ||
onDismissWithDeviceSelected: { [weak self] device in | ||
self?.previewViewSelectedPreviewDevice = device | ||
}, | ||
completion: completion | ||
) | ||
|
||
let navController = GutenbergLightNavigationController(rootViewController: previewVC) | ||
navController.modalPresentationStyle = .pageSheet | ||
navigationController?.present(navController, animated: true) | ||
} | ||
|
||
func didDeselectItem(forCell cell: CategorySectionTableViewCell) {} | ||
|
||
func accessibilityElementDidBecomeFocused(forCell cell: CategorySectionTableViewCell) { | ||
guard UIAccessibility.isVoiceOverRunning, let cellIndexPath = tableView.indexPath(for: cell) else { return } | ||
tableView.scrollToRow(at: cellIndexPath, at: .middle, animated: true) | ||
} | ||
} |
19 changes: 19 additions & 0 deletions
19
WordPress/Classes/ViewRelated/Site Creation/Design Selection/SiteDesignSection.swift
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
import Foundation | ||
|
||
class SiteDesignSection: CategorySection { | ||
var category: RemoteSiteDesignCategory | ||
var designs: [RemoteSiteDesign] | ||
|
||
var categorySlug: String { category.slug } | ||
var title: String { category.title } | ||
var emoji: String? { category.emoji } | ||
var description: String? { category.description } | ||
var thumbnails: [Thumbnail] { designs } | ||
var scrollOffset: CGPoint | ||
|
||
init(category: RemoteSiteDesignCategory, designs: [RemoteSiteDesign]) { | ||
self.category = category | ||
self.designs = designs | ||
self.scrollOffset = .zero | ||
} | ||
} |
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
Oops, something went wrong.
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.
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.
if
set
does nothing, can we justThere 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.
Good catch! I'll get this fixed. This was left over from being a subclass of
FilterableCategoriesViewController
which required a setter. 👍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.
oh I see 👍
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.
This should be resolved with this commit.