Skip to content
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

Quick Start for Existing Users: Add media upload tour #18471

Merged
merged 20 commits into from
May 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
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
2 changes: 1 addition & 1 deletion Podfile
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ abstract_target 'Apps' do

pod 'NSURL+IDN', '~> 0.4'

pod 'WPMediaPicker', '~> 1.8.3'
pod 'WPMediaPicker', '~> 1.8.4-beta.1'
# pod 'WPMediaPicker', :git => 'https://github.com/wordpress-mobile/MediaPicker-iOS.git', :tag => '1.7.0'
## while PR is in review:
# pod 'WPMediaPicker', :git => 'https://github.com/wordpress-mobile/MediaPicker-iOS.git', :branch => ''
Expand Down
8 changes: 4 additions & 4 deletions Podfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,7 @@ PODS:
- CocoaLumberjack (~> 3.4)
- FormatterKit/TimeIntervalFormatter (~> 1.8)
- WordPressUI (1.12.5)
- WPMediaPicker (1.8.3)
- WPMediaPicker (1.8.4-beta.1)
- wpxmlrpc (0.9.0)
- Yoga (1.14.0)
- ZendeskCommonUISDK (6.1.2)
Expand Down Expand Up @@ -595,7 +595,7 @@ DEPENDENCIES:
- WordPressMocks (~> 0.0.15)
- WordPressShared (~> 1.17.1)
- WordPressUI (~> 1.12.5)
- WPMediaPicker (~> 1.8.3)
- WPMediaPicker (~> 1.8.4-beta.1)
- Yoga (from `https://raw.githubusercontent.com/wordpress-mobile/gutenberg-mobile/v1.75.0/third-party-podspecs/Yoga.podspec.json`)
- ZendeskSupportSDK (= 5.3.0)
- ZIPFoundation (~> 0.9.8)
Expand Down Expand Up @@ -857,7 +857,7 @@ SPEC CHECKSUMS:
WordPressMocks: 6b52b0764d9939408151367dd9c6e8a910877f4d
WordPressShared: 0c4bc5e25765732fcf5d07f28c81970ab28493fb
WordPressUI: c5be816f6c7b3392224ac21de9e521e89fa108ac
WPMediaPicker: 0f4f20c7f661b46d33283f1ac2adceb98718fffa
WPMediaPicker: 547b19abb9361a0a70ac7df4399502b3d59f1f3b
wpxmlrpc: bf55a43a7e710bd2a4fb8c02dfe83b1246f14f13
Yoga: 2ca978c40e0fd6d7f54bcb1602bc0cbbc79454a7
ZendeskCommonUISDK: 5f0a83f412e07ae23701f18c412fe783b3249ef5
Expand All @@ -869,6 +869,6 @@ SPEC CHECKSUMS:
ZendeskSupportSDK: 3a8e508ab1d9dd22dc038df6c694466414e037ba
ZIPFoundation: ae5b4b813d216d3bf0a148773267fff14bd51d37

PODFILE CHECKSUM: a9583cec2bc246d23cf0045230cd179dc03c5adb
PODFILE CHECKSUM: f6bd23447af8d4f331d87b63590f4bf520730633

COCOAPODS: 1.11.2
2 changes: 1 addition & 1 deletion RELEASE-NOTES.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
19.9
-----

* [*] Media Picker: Fixed an issue where the empty state view was being displayed incorrectly. [#18471]

19.8
-----
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ final class BlogDashboardViewController: UIViewController {
switch element {
case .setupQuickStart, .removeQuickStart:
self.loadCardsFromCache()
case .stats:
case .stats, .mediaScreen:
if self.embeddedInScrollView {
self.mySiteScrollView?.scrollToTop(animated: true)
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,18 +138,23 @@ extension DashboardQuickActionsCardCell {
let element = info[QuickStartTourGuide.notificationElementKey] as? QuickStartTourElement {

switch element {
case .noSuchElement:
self?.statsButton.shouldShowSpotlight = false
case .stats:
guard QuickStartTourGuide.shared.entryPointForCurrentTour == .blogDashboard else {
return
}

self?.statsButton.shouldShowSpotlight = true
self?.autoScrollToStatsButton()
case .mediaScreen:
guard QuickStartTourGuide.shared.entryPointForCurrentTour == .blogDashboard else {
return
}

self?.autoScrollToMediaButton()
default:
break
}
self?.statsButton.shouldShowSpotlight = element == .stats
self?.mediaButton.shouldShowSpotlight = element == .mediaScreen
}
}
}
Expand All @@ -161,6 +166,10 @@ extension DashboardQuickActionsCardCell {
private func autoScrollToStatsButton() {
scrollView.scrollHorizontallyToView(statsButton, animated: true)
}

private func autoScrollToMediaButton() {
scrollView.scrollHorizontallyToView(mediaButton, animated: true)
}
}

extension DashboardQuickActionsCardCell {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ extension BlogDetailsViewController {

if let info = notification.userInfo?[QuickStartTourGuide.notificationElementKey] as? QuickStartTourElement {
switch info {
case .stats:
case .stats, .mediaScreen:
guard QuickStartTourGuide.shared.entryPointForCurrentTour == .blogDetails else {
return
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ typedef NS_ENUM(NSInteger, QuickStartTourElement) {
QuickStartTourElementNotifications = 22,
QuickStartTourElementSetupQuickStart = 23,
QuickStartTourElementRemoveQuickStart = 24,
QuickStartTourElementMediaScreen = 25,
QuickStartTourElementMediaUpload = 26,
};

typedef NS_ENUM(NSUInteger, BlogDetailsNavigationSource) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -900,6 +900,7 @@ - (BlogDetailsSection *)publishTypeSectionViewModel
callback:^{
[weakSelf showMediaLibraryFromSource:BlogDetailsNavigationSourceRow];
}];
mediaRow.quickStartIdentifier = QuickStartTourElementMediaScreen;
[rows addObject:mediaRow];

BlogDetailsRow *pagesRow = [[BlogDetailsRow alloc] initWithTitle:NSLocalizedString(@"Pages", @"Noun. Title. Links to the blog's Pages screen.")
Expand Down Expand Up @@ -1437,7 +1438,7 @@ - (void)showMediaLibraryFromSource:(BlogDetailsNavigationSource)source
controller.navigationItem.largeTitleDisplayMode = UINavigationItemLargeTitleDisplayModeNever;
[self showDetailViewController:controller sender:self];

[[QuickStartTourGuide shared] visited:QuickStartTourElementBlogDetailNavigation];
[[QuickStartTourGuide shared] visited:QuickStartTourElementMediaScreen];
}

- (void)showPeople
Expand Down
26 changes: 26 additions & 0 deletions WordPress/Classes/ViewRelated/Blog/QuickStartTours.swift
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,32 @@ struct QuickStartNotificationsTour: QuickStartTour {
let accessibilityHintText = NSLocalizedString("Guides you through the process of checking your notifications.", comment: "This value is used to set the accessibility hint text for viewing the user's notifications.")
}

struct QuickStartMediaUploadTour: QuickStartTour {
let key = "quick-start-media-upload-tour"
let analyticsKey = "media"
let title = NSLocalizedString("Upload photos or videos", comment: "Title of a Quick Start Tour")
let titleMarkedCompleted = NSLocalizedString("Completed: Upload photos or videos", comment: "The Quick Start Tour title after the user finished the step.")
let description = NSLocalizedString("Bring media straight from your device or camera to your site.", comment: "Description of a Quick Start Tour")
let icon = UIImage.gridicon(.addImage)
let suggestionNoText = Strings.notNow
let suggestionYesText = Strings.yesShowMe
let possibleEntryPoints: Set<QuickStartTourEntryPoint> = [.blogDetails, .blogDashboard]

var waypoints: [WayPoint] = {
let step1DescriptionBase = NSLocalizedString("Select %@ to see your current library.", comment: "A step in a guided tour for quick start. %@ will be the name of the item to select.")
let step1DescriptionTarget = NSLocalizedString("Media", comment: "The menu item to select during a guided tour.")
let step1: WayPoint = (element: .mediaScreen, description: step1DescriptionBase.highlighting(phrase: step1DescriptionTarget, icon: .gridicon(.image)))

let step2DescriptionBase = NSLocalizedString("Select %@to upload media. You can add it to your posts / pages from any device.", comment: "A step in a guided tour for quick start. %@ will be a plus icon.")
Copy link
Contributor

Choose a reason for hiding this comment

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

[Q] Is it intentional that there no space between the "%@" and the "to"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is yes.
String.highlighting adds a space between the title and the icon passed. Given that the title is empty in this case, this means String.highlighting will add an extra space either way.

Removing the space from the base sting was to mitigate this issue. This was simpler than modifying String.highlighting to handle this specific case.

let step2DescriptionTarget = ""
let step2: WayPoint = (element: .mediaUpload, description: step2DescriptionBase.highlighting(phrase: step2DescriptionTarget, icon: .gridicon(.plus)))

return [step1, step2]
}()

let accessibilityHintText = NSLocalizedString("Guides you through the process of uploading new media.", comment: "This value is used to set the accessibility hint text for viewing the user's notifications.")
}

private extension String {
func highlighting(phrase: String, icon: UIImage?) -> NSAttributedString {
let normalParts = components(separatedBy: "%@")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ struct QuickStartGetToKnowAppCollection: QuickStartToursCollection {
QuickStartCheckStatsTour(),
QuickStartNotificationsTour(),
QuickStartViewTour(blog: blog),
QuickStartMediaUploadTour(),
QuickStartFollowTour()
]
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ class MediaLibraryViewController: WPMediaPickerViewController {

fileprivate var isLoading: Bool = false
fileprivate let noResultsView = NoResultsViewController.controller()
fileprivate let addButton: SpotlightableButton = SpotlightableButton(type: .custom)

fileprivate var kvoTokens: [NSKeyValueObservation]?

fileprivate var selectedAsset: Media? = nil

Expand Down Expand Up @@ -62,12 +65,13 @@ class MediaLibraryViewController: WPMediaPickerViewController {
controller.navigationItem.largeTitleDisplayMode = .never
sourceController.navigationController?.pushViewController(controller, animated: true)

QuickStartTourGuide.shared.visited(.blogDetailNavigation)
QuickStartTourGuide.shared.visited(.mediaScreen)
}

deinit {
unregisterChangeObserver()
unregisterUploadCoordinatorObserver()
stopObservingNavigationBarClipsToBounds()
}

private class func pickerOptions() -> WPMediaPickerOptions {
Expand Down Expand Up @@ -104,6 +108,9 @@ class MediaLibraryViewController: WPMediaPickerViewController {
if let collectionView = collectionView {
WPStyleGuide.configureColors(view: view, collectionView: collectionView)
}

navigationController?.navigationBar.subviews.forEach ({ $0.clipsToBounds = false })
startObservingNavigationBarClipsToBounds()
}

override func viewDidAppear(_ animated: Bool) {
Expand All @@ -120,6 +127,11 @@ class MediaLibraryViewController: WPMediaPickerViewController {
}
}

override func viewWillAppear(_ animated: Bool) {
super.viewWillAppear(animated)
addButton.shouldShowSpotlight = QuickStartTourGuide.shared.isCurrentElement(.mediaUpload)
}

// MARK: - Update view state

fileprivate func updateViewState(for assetCount: Int) {
Expand All @@ -142,12 +154,18 @@ class MediaLibraryViewController: WPMediaPickerViewController {

var barButtonItems = [UIBarButtonItem]()

if blog.userCanUploadMedia && assetCount > 0 {
let addButton = UIBarButtonItem(barButtonSystemItem: .add, target: self, action: #selector(addTapped))
if blog.userCanUploadMedia {
addButton.spotlightOffset = Constants.addButtonSpotlightOffset
let config = UIImage.SymbolConfiguration(textStyle: .body, scale: .large)
let image = UIImage(systemName: "plus", withConfiguration: config) ?? .gridicon(.plus)
Copy link
Contributor

Choose a reason for hiding this comment

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

[Q] Any particular reason we're using the system image over the gridicon?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We were using the system's default button. Since I had to manually create the button to add the spotlight view, I tried (to the best of my ability) to maintain the same look as before. I found that the plus grid icon was a bit thicker than the system icon.

Grid Icon System
GridIcon System

addButton.setImage(image, for: .normal)
addButton.contentEdgeInsets = Constants.addButtonContentInset
addButton.addTarget(self, action: #selector(addTapped), for: .touchUpInside)
addButton.accessibilityLabel = NSLocalizedString("Add", comment: "Accessibility label for add button to add items to the user's media library")
addButton.accessibilityHint = NSLocalizedString("Add new media", comment: "Accessibility hint for add button to add items to the user's media library")

barButtonItems.append(addButton)
let addBarButton = UIBarButtonItem(customView: addButton)
barButtonItems.append(addBarButton)
}

if blog.supports(.mediaDeletion) && assetCount > 0 {
Expand All @@ -157,10 +175,8 @@ class MediaLibraryViewController: WPMediaPickerViewController {

barButtonItems.append(editButton)

navigationItem.setRightBarButtonItems(barButtonItems, animated: false)
} else {
navigationItem.setRightBarButtonItems(barButtonItems, animated: false)
}
navigationItem.setRightBarButtonItems(barButtonItems, animated: false)
}
}

Expand Down Expand Up @@ -255,6 +271,8 @@ class MediaLibraryViewController: WPMediaPickerViewController {
// MARK: - Actions

@objc fileprivate func addTapped() {
QuickStartTourGuide.shared.visited(.mediaUpload)
addButton.shouldShowSpotlight = QuickStartTourGuide.shared.isCurrentElement(.mediaUpload)
showOptionsMenu()
}

Expand Down Expand Up @@ -427,6 +445,25 @@ class MediaLibraryViewController: WPMediaPickerViewController {
MediaCoordinator.shared.removeObserver(withUUID: uuid)
}
}

// MARK: ClipsToBounds KVO Observer

/// The content view of the navigation bar causes the spotlight view on the add button to be clipped.
/// This ensures that `clipsToBounds` of the content view is always `false`.
/// Without this, `clipsToBounds` reverts to `true` at some point during the view lifecycle. This happens asynchronously,
/// so we can't confidently reset it. Hence the need for KVO.
private func startObservingNavigationBarClipsToBounds() {
kvoTokens = navigationController?.navigationBar.subviews.map({ subview in
return subview.observe(\.clipsToBounds, options: .new, changeHandler: { view, change in
guard let newValue = change.newValue, newValue else { return }
view.clipsToBounds = false
})
})
}

private func stopObservingNavigationBarClipsToBounds() {
kvoTokens?.forEach({ $0.invalidate() })
}
}

// MARK: - UIDocumentPickerDelegate
Expand Down Expand Up @@ -742,3 +779,12 @@ extension MediaLibraryViewController: TenorPickerDelegate {
}
}
}

// MARK: Constants

extension MediaLibraryViewController {
private enum Constants {
static let addButtonSpotlightOffset = UIOffset(horizontal: 20, vertical: -10)
static let addButtonContentInset = UIEdgeInsets(top: 0, left: 10, bottom: 0, right: 0)
}
}
5 changes: 3 additions & 2 deletions WordPress/WordPressTest/QuickStartFactoryTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,12 @@ class QuickStartFactoryTests: XCTestCase {
let tours = QuickStartFactory.allTours(for: blog)

// Then
XCTAssertEqual(tours.count, 4)
XCTAssertEqual(tours.count, 5)
XCTAssertTrue(tours[0] is QuickStartCheckStatsTour)
XCTAssertTrue(tours[1] is QuickStartNotificationsTour)
XCTAssertTrue(tours[2] is QuickStartViewTour)
XCTAssertTrue(tours[3] is QuickStartFollowTour)
XCTAssertTrue(tours[3] is QuickStartMediaUploadTour)
XCTAssertTrue(tours[4] is QuickStartFollowTour)
}

func testToursForNewSite() {
Expand Down