Top Performers Mark3: User Interface #305

bummytime · 2018-09-14T16:25:47Z

This PR wraps up the work on Top Performers. It mainly contains UI additions:

Just like NewOrdersViewController, the top performers table uses a ResultsController instance not attached to a tableview. Updates should be reflects as soon as new data is inserted into Storage.

Other Misc things:

In Array+Helpers.swift added a helper func to safely grab a value from a Collection regardless of bounds
In AppDelegate.swift, updated the UITableViewHeaderFooterView text color via appearance proxy (the Orders List screen had the wrong color for section headers)
In UIImage+Woo.swift, added a GridIcon that i used for a product image placeholder if a valid URL is not available
Made buildDateString() a static func on StatsStore
Remote product images are brought to you by UIImageView+Networking in WordPressUI 🎉

Also note that I addressed the "stale data after logout" issue (#300) in this PR. Both New Orders and Top performers should correctly reset after the user logs out. The revenue chart data does NOT reset correctly after a log out, however this will be addressed when #258 is completed.

Fixes: #178
Fixes #300

Testing

Preconditions: Start with a fresh install of the app. Log into a your store of choice (ideally one with recent-ish orders).

Give the UI a good test drive here. Things to consider:

Portrait and landscape orientations
Multiple device sizes
UI updates (Pull to refresh) after adding/updating/deleting orders from the store you are logged into
Logout of one store and sign into another — does everything work like you expect?

@jleandroperez and @mindgraffiti would you mind taking this for a spin? Thanks!

… feature/178-top-performers-mark3 * 'develop' of github.com:woocommerce/woocommerce-ios: Increasing version to Mark 0.7 Clear barChart data after period VC disappears

… feature/178-top-performers-mark3 * 'develop' of github.com:woocommerce/woocommerce-ios: (25 commits) SyncingCoordinator: Adding Documentation Note! OrdersViewController: Updating pageSize SyncCoordinator: Fixing faulty logic OrdersViewController: Decreasing the pageSize OrdersViewController: Restores pageSize Revert to this commit to test PR 295 Implements SyncingCoordinator Unit Tests OrdersViewController: New FSM State NewOrdersViewController: Updates constants SyncCoordinator: Renames Public API SyncCoordinator: Fixing condition OrdersViewController: SyncingCoordinatorDelegate Conformance SyncCoordinator: Documenting API NewOrdersViewController: Wiring updated synchronizeOrders Action OrderAction: Page > PageNumber OrderAction: Renames parameters OrdersViewController: Wiring Sync Coordinator Implements SyncingCoordinator NewOrdersViewController: Wiring new Sync API OrderAction: Synchronize Orders now expects pageSize and pageNumber ...

… feature/178-top-performers-mark3 * 'develop' of github.com:woocommerce/woocommerce-ios: OrdersViewController: Reverts Testing Code Revert to this commit to test Error State Revert to this commit to test Empty Unfiltered State OrdersViewController: Notice as Error Handler OrdersViewController: Nukes FSM Error State

wpmobilebot · 2018-09-14T16:34:39Z

	1 Warning
⚠️	PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 Danger

jleandroperez

@bummytime code looks great!! few issues i've hit:

Issue A: Suck in No Activity

Log into a WC Store
Open the Dashboard
Top Performers display No Activity
Buy an item (on the web)
Pull to refresh

As a result, i'm stuck in the No Activity state.

I'm triggering this in (my) Store. Julia's works just fine... am i missing something? is there a minimum sale threshold?

Issue B: Logout > Not Sync'ing

Log into Account A
Logout
Log into Account B

I'm still having the "Dashboard not syncing" (after an account switch).

Issue C: Pull to Refresh Flicker

Open the Dashboard
Pull to Refresh

As a result, the spinner is hidden immediately (zero delay). Feels a bit off, maybe we could do something like what we have in OrderDetailsViewController? (dispatch group + multiple sync OPs)

Issue D: Empty State Bottom Padding

This is not an issue, actually, a detail that we need to spin over!.

Whenever (everything) is in the Empty State, the bottom padding between the placeholder asset and the edge of the screen is huuuge. Maybe we could:

A. Reduce the bottom padding?
B. Add top padding so that it's... balanced?

Still, any alternative i could come up falls short: whenever any of the tabs happens not to be empty... well, the assymetry will be even worse right?.

Please let me know if i did something wrong, the first issue (empty state in my blog) feels like there is something i'm not doing?

Thanks Matt, looks amazing!!!

jleandroperez · 2018-09-14T17:47:33Z

WooCommerce/Classes/ViewRelated/Dashboard/MyStore/Cells/ProductTableViewCell.swift

+        if let productURLString = statsItem?.imageUrl {
+            productImage.downloadImage(from: URL(string: productURLString), placeholderImage: UIImage.productPlaceholderImage)
+        } else {
+            productImage.image = UIImage.productPlaceholderImage


(No need to change anything, just passing along an awesome trick!!) You can just do as follows, because the UIImage is inferred from the context:

productImage.image = .productPlaceholderImage

bummytime · 2018-09-14T21:30:40Z

Thanks for the thorough review @jleandroperez!

Issue A: Suck in No Activity

We chatted offline about this and "fixed the glitch" on the site itself ;-)

Issue B: Logout > Not Sync'ing

Yeah, this is a known issue and will get fixed in #260 (before open beta!)

Issue C: Pull to Refresh Flicker

Good catch, I will see if I can address this here without a ton of code. If not I may open another issue.

Issue D: Empty State Bottom Padding

Yeah, I am not satisfied with the current state of this either. I will update it in this PR!

mindgraffiti

Looking good!

Minor edits needed for some code comments. Needs a little polish to the line height on the Top Performers name label. I know, I know, autolayout :shakesfistatsky: :)

mindgraffiti · 2018-09-17T13:50:54Z

WooCommerce/Classes/Extensions/Array+Helpers.swift

+
+extension Collection {
+
+    /// Returns the element at the specified index iff it is within bounds, otherwise nil.


extra f in if

@mindgraffiti That is the old programmer in me — I meant to do that (meaning: "if and only if..." ). I'll remove it for clarity. 😉

TIL there is notation for "if and only if". please keep that!

mindgraffiti · 2018-09-17T13:58:11Z

WooCommerce/Classes/Model/TopEarnerStatsItem+Woo.swift

+        return Locale(identifier: identifier).currencySymbol ?? currency
+    }
+
+    /// Returns the a friendly-formatted total string including the currency symbol


delete "the"

mindgraffiti · 2018-09-17T13:58:47Z

WooCommerce/Classes/Model/TopEarnerStatsItem+Woo.swift

+    /// Returns the a friendly-formatted total string including the currency symbol
+    ///
+    var formattedTotalString: String {
+        return currencySymbol + total.friendlyString()


Once I'm done with the MoneyFormatter struct, we'll have new toys to use!

mindgraffiti · 2018-09-17T14:34:07Z

WooCommerce/Classes/ViewRelated/Dashboard/MyStore/Cells/ProductTableViewCell.xib

@@ -0,0 +1,77 @@
+<?xml version="1.0" encoding="UTF-8"?>


Not certain what's going on here, but the nameLabel has too much height / padding / line height. I tried switching the label to multiline, checking for height constraints, and checking the stackview (but none of that is fixing the issue).

mindgraffiti · 2018-09-17T14:36:06Z

WooCommerce/Classes/AppDelegate.swift

        UINavigationBar.appearance().isTranslucent = false
        UINavigationBar.appearance().tintColor = .white
        UIApplication.shared.statusBarStyle = .lightContent
+        UILabel.appearance(whenContainedInInstancesOf: [UITableViewHeaderFooterView.self]).textColor = StyleManager.sectionTitleColor


wpmobilebot · 2018-09-17T20:29:40Z

WooCommerce/Classes/ViewRelated/ReusableViews/IntrinsicTableView.swift

+
+class IntrinsicTableView: UITableView {
+
+    override var contentSize:CGSize {


⚠️ Colons should be next to the identifier when specifying a type and next to the key in dictionary literals.

jleandroperez

Looking great!!! discussed over slack, we're addressing the pendings in smaller issues, we need to move this puppy!!

Thanks Bummy!!!

bummytime · 2018-09-17T21:00:03Z

Thanks @jleandroperez — issues are logged!

jleandroperez · 2018-09-18T02:05:46Z

Thanks for doing that sir!!! 🙇

peril-woocommerce · 2021-05-06T00:44:54Z

	Warnings
⚠️	PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS

bummytime added 21 commits September 11, 2018 12:34

WIP: Dashboard top performers UI

ca0ee45

Top Perf: Added section label

817e671

Merge branch 'develop' of github.com:woocommerce/woocommerce-ios into…

b5c8cd9

… feature/178-top-performers-mark3 * 'develop' of github.com:woocommerce/woocommerce-ios: Increasing version to Mark 0.7 Clear barChart data after period VC disappears

Top Perf: Fixed a spacing issue

6dc3cc5

Top Perf: Layout adjustments

e7ebf1f

Top Perf: Fixed some constraints

36c2546

Top Perf: Added tab titles

87bbb52

Removed superfluous VC from dash storyboard

df1a40c

Top Perf section headers

02da5b2

Top perf tweaks

7f77f03

Added ProductTableViewCell

03d4883

WIP: Loading data in top performers

c5ef920

Top Perf: UI updates

49717af

ProductTableViewCell updates

9bcb8a9

Top Perf: Wired up ResultsController

038c370

Top Perf: Added formatted price string to table

78e9cef

Top Perf: Sorting

0e43031

Dashboard: resultsController.onDidResetContent imp'ed

48e675a

Top Perf: Added no results view

758166f

bummytime added the Epic: Dashboard label Sep 14, 2018

bummytime added this to the External open beta milestone Sep 14, 2018

bummytime self-assigned this Sep 14, 2018

bummytime requested review from jleandroperez and mindgraffiti September 14, 2018 16:25

jleandroperez reviewed Sep 14, 2018

View reviewed changes

Lint fixes!

b023664

mindgraffiti reviewed Sep 17, 2018

View reviewed changes

bummytime added 3 commits September 17, 2018 10:27

Top perf: leave section header in place if table is empty

a1bfc38

Top Perf: doc comment updates

62bc388

Added IntrinsicTableView subclass

d7f9513

wpmobilebot reviewed Sep 17, 2018

View reviewed changes

jleandroperez approved these changes Sep 17, 2018

View reviewed changes

bummytime merged commit bc42e5b into develop Sep 17, 2018

bummytime deleted the feature/178-top-performers-mark3 branch September 17, 2018 20:46

This was referenced Sep 17, 2018

Top Performers: Support dynamic text #308

Closed

Dashboard: Fix pull to refresh #309

Closed

designsimply added feature: stats Related to stats, including Top Performers. and removed [Feat] Dashboard labels May 6, 2021

AliSoftware added feature: stats Related to stats, including Top Performers. and removed feature: stats Related to stats, including Top Performers. labels May 6, 2021


		extension Collection {

		/// Returns the element at the specified index iff it is within bounds, otherwise nil.


		class IntrinsicTableView: UITableView {

		override var contentSize:CGSize {

Top Performers Mark3: User Interface #305

Top Performers Mark3: User Interface #305

Uh oh!

Conversation

bummytime commented Sep 14, 2018 • edited by mindgraffiti Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Testing

Uh oh!

wpmobilebot commented Sep 14, 2018

Uh oh!

jleandroperez left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Issue A: Suck in No Activity

Issue B: Logout > Not Sync'ing

Issue C: Pull to Refresh Flicker

Issue D: Empty State Bottom Padding

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bummytime commented Sep 14, 2018

Uh oh!

mindgraffiti left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jleandroperez left a comment

Choose a reason for hiding this comment

Uh oh!

bummytime commented Sep 17, 2018

Uh oh!

jleandroperez commented Sep 18, 2018

Uh oh!

peril-woocommerce bot commented May 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

bummytime commented Sep 14, 2018 •

edited by mindgraffiti

Loading

jleandroperez left a comment •

edited

Loading

peril-woocommerce bot commented May 6, 2021 •

edited

Loading