Update card design and do not display author name when "Me" is selected #11705

leandroalonso · 2019-05-15T22:33:01Z

Ref #11646

In this PR:

The post card was redesigned;
The author is not displayed when "Me" is selected as a filter;
Fixed a bug that was displaying a large empty area at the top of the posts lists

To test:

Use the blueprint in the master issue as a starting point. These are some user-scenarios that can be tested:

Check how Published, Drafted, Scheduled and Trashed posts look
Make sure that when selecting "Me" your name is not displayed
Trash some posts, then Undo
Edit posts and check that they update
Test in larger devices (ie.: iPad) as well with increased font sizes
Upload media and check that the progress bar is correctly displayed
Remove your internet connection, change a post and make sure that a "Retry" button is displayed

Here are some screenshots:

Update release notes:

If there are user facing changes, I have added an item to RELEASE-NOTES.txt.

WordPress/Classes/ViewRelated/Post/PostCell.swift

peril-wordpress-mobile · 2019-05-15T22:33:07Z

	Warnings
⚠️	PR is not assigned to a milestone.
⚠️	PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS

SylvesterWilmott · 2019-05-17T09:33:17Z

Looks really good @leandroalonso thanks!

Can we modify the height of the card images for iPad? If you check in the current reader, we should re-use the height of the images there for iPad.

SylvesterWilmott · 2019-05-20T11:53:10Z

Thanks for the changes @leandroalonso

Looks great!

nheagy

This is looking really good. Replacing the post cell(s) is a fairly big undertaking, so I appreciate the courage :D

Besides what I've mentioned in the code, there are a few issues in RtL languages:

Also, after changing text size the text does update, but images disappear:

And feel free to let me know when you think my comments are entirely wrong 😆

nheagy · 2019-05-22T20:00:29Z

WordPress/Classes/Extensions/UIView+layoutMargin.swift

+import Foundation
+
+extension UIView {
+    func setLayoutMargin(top: CGFloat? = nil, left: CGFloat? = nil, bottom: CGFloat? = nil, right: CGFloat? = nil) {


setLayoutMargin should be plural, but setLayoutMargins already exists. Want to take another pass at this method name?

Changed to changeLayoutMargins.

nheagy · 2019-05-22T20:02:25Z

WordPress/Classes/Extensions/UIView+layoutMargin.swift

@@ -0,0 +1,12 @@
+import Foundation


Our naming scheme for extension files is to capitalize the name after the +, such as UIView+LayoutMargins.swift. However, I feel like UIView+Helpers.swift in WordPressUI is a better fit for this than adding a new file to the WPiOS project.

Moved: wordpress-mobile/WordPressUI-iOS#34

nheagy · 2019-05-22T20:26:31Z

WordPress/Classes/Models/AbstractPost.m

-        NSString *lastModified = NSLocalizedString(@"last-modified",@"A label for a post's last-modified date.");
-        return [NSString stringWithFormat:@"%@ (%@)", shortDate, lastModified];
-    } else if ([self isScheduled]) {
+    if ([self isScheduled]) {


This looks to be removing the last modified date from drafts and pending. Is that intentional?

My bad. The idea was to remove the last-modified string, not the entire date. Fixed.

WordPress/Classes/ViewRelated/Post/AbstractPostListViewController.swift

nheagy · 2019-05-22T20:29:37Z

WordPress/Classes/ViewRelated/Post/PostCardStatusViewModel.swift

@@ -4,9 +4,10 @@ import Gridicons
 /// Encapsulates status display logic for PostCardTableViewCells.
 ///
 class PostCardStatusViewModel: NSObject {
+    private let separator = "·"


We put constants like this in a Constants enum. You'll find examples if you search the codebase for private enum Constants

nheagy · 2019-05-22T20:44:57Z

WordPress/Classes/ViewRelated/Post/RestorePostTableViewCell.m

@@ -41,7 +41,7 @@ - (void)applyStyles
    [WPStyleGuide applyRestorePostButtonStyle:self.restoreButton];

    self.postContentView.layer.borderColor = [[WPStyleGuide postCardBorderColor] CGColor];
-    self.postContentView.layer.borderWidth = 1.0;
+    self.postContentView.layer.borderWidth = 0.5;


Do we want 0.5 or do we want 1px in the devices display resolution?

Good catch. Refactored to ensure 1px in devices display resolution. Also changed in PostCell.

nheagy · 2019-05-22T20:50:19Z

WordPress/WordPressTest/PostCellTests.swift

+
+@testable import WordPress
+
+class PostCellTests: XCTestCase {


Loving the tests.

nheagy · 2019-05-22T21:05:28Z

WordPress/Classes/ViewRelated/Post/PostCell.swift

+    @IBOutlet weak var titleAndSnippetView: UIStackView!
+    @IBOutlet weak var topMargin: NSLayoutConstraint!
+
+    private let margin: CGFloat = WPDeviceIdentification.isiPad() ? 20 : 16


Let's put all these constants in a constants enum (or two)

nheagy · 2019-05-22T21:05:54Z

WordPress/Classes/ViewRelated/Post/PostCell.swift

+        return ImageLoader(imageView: featuredImage, gifStrategy: .mediumGIFs)
+    }()
+
+    private var post: Post!


Can we do this without the !s?

By making it Optional we will need to add boilerplate guard in 9 different places. Also, all the methods that use post are called only after a post is assigned.

What do you think? Keep the ! or change it to ??

I'm solidly in the ? camp. This is something our team has debates both sides for—sadly you're caught on my side at the moment 😆

nheagy · 2019-05-22T21:10:09Z

WordPress/Classes/ViewRelated/Post/PostCell.swift

+        setNeedsDisplay()
+    }
+
+    override func hitTest(_ point: CGPoint, with event: UIEvent?) -> UIView? {


Can you explain the need for a hitTest?

This reverts commit d0629e065da26c94f670ea1baa3fbfad3d730dde.

nheagy

I feel like this is in a really good spot!

I have one request regarding the separator 😆

nheagy · 2019-05-27T23:03:50Z

WordPress/Classes/ViewRelated/Post/PostCardStatusViewModel.swift

@@ -140,6 +140,12 @@ class PostCardStatusViewModel: NSObject {
        }
    }

+    func author(separatorDirection direction: UIUserInterfaceLayoutDirection) -> String {


This separator logic feels overdone. Could we find a simpler way to make this look right? What about checking the mockups again and seeing if there's a better place to add the separator than in the ViewModel for the author label's text?

Done. I've extracted the separator itself to the view controller.

WordPress/Classes/ViewRelated/Post/PostCell.swift

nheagy

Nice! 🎉

houndci-bot reviewed May 15, 2019

View reviewed changes

WordPress/Classes/ViewRelated/Post/PostCell.swift Outdated Show resolved Hide resolved

WordPress/Classes/ViewRelated/Post/PostCell.swift Outdated Show resolved Hide resolved

nheagy added [Status] Needs Design Review A designer needs to sign off on the implemented design. Posts List labels May 15, 2019

nheagy assigned leandroalonso May 15, 2019

leandroalonso mentioned this pull request May 15, 2019

Post List Revamp Master Issue #11646

Closed

8 tasks

SylvesterWilmott removed the [Status] Needs Design Review A designer needs to sign off on the implemented design. label May 20, 2019

nheagy suggested changes May 22, 2019

View reviewed changes

nheagy force-pushed the issues/11646-post-list-revamp branch from bfef387 to 4d6ada6 Compare May 23, 2019 21:05

leandroalonso added 19 commits May 23, 2019 19:52

Creates a PostCell with the new layout arrangement

9fe8ef3

Shows featuredImage if available, otherwise hide the image placeholder

bf3085f

Apply post card styles and set up the borders

d5e4ec5

Configure post cell title

1ec3679

Change top space accordingly with image availability

12297d9

Show post's snippet

9c77a68

Display post date

9f3983d

Show post's author

e7fe116

Show if post is sticky

d16617b

Show if post is private

36ead2e

Configure post status and separator if needed

20d1556

Show status view with correct colors and proper separator logic

223251e

Show progress bar

01778d9

Apply styles to status view labels

a11899c

Adds view and edit actions

55259a1

Display action sheet when more is tapped

7e3a899

Add retry action when post is failed

2e09433

When using a filter to show only mine posts, hide author

c332fa2

Do not display "last-modified" for post dates

1594e01

leandroalonso added 20 commits May 23, 2019 19:57

Margin and styles improvements

7cf9775

Call super traitCollectionDidChange to avoid issues

8e8e632

Fix action bar height

21893b2

Few renames for better readability

e674660

Refactor to simplify some constants

caf9c7f

Adds a horizontal stack view for the upcoming compact mode

0d89ae5

Revert "Adds a horizontal stack view for the upcoming compact mode"

170bc37

This reverts commit d0629e065da26c94f670ea1baa3fbfad3d730dde.

Moves some logics to the view model

485542c

Adds back the last modified date for drafts and pending

7c7f53e

Extract the separator into a constants enum

a536362

Move trash and scheduled changes to statusForDisplay

896011f

Changes view model for better understanding

7394c00

Ensure that borders has 1px of devices display resolution

53ebde3

Add a comment in hit test

892c020

Extract constants

b6305c6

Fix issues with right-to-left direction and with images

81903cf

Rename setLayoutMargin to changeLayoutMargins

60afe28

Remove unused xib

4410867

Fix typo

96c2c46

Moves changeLayoutMargins to WordPressUI

89c1157

nheagy suggested changes May 27, 2019

View reviewed changes

leandroalonso added 2 commits May 27, 2019 21:42

Remove the space from the view model and added in the view itself

249858c

Makes post and view model optional

da7287d

houndci-bot reviewed May 28, 2019

View reviewed changes

WordPress/Classes/ViewRelated/Post/PostCell.swift Outdated Show resolved Hide resolved

houndci-bot reviewed May 28, 2019

View reviewed changes

WordPress/Classes/ViewRelated/Post/PostCell.swift Outdated Show resolved Hide resolved

leandroalonso added 2 commits May 27, 2019 21:51

Removes trailing whitespace

5c117dc

Rename isAllPostsMine

e1d57ab

leandroalonso mentioned this pull request May 28, 2019

Display "ghost post card" when loading posts #11789

Merged

nheagy approved these changes May 28, 2019

View reviewed changes

nheagy merged commit bdac32d into wordpress-mobile:issues/11646-post-list-revamp May 28, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update card design and do not display author name when "Me" is selected #11705

Update card design and do not display author name when "Me" is selected #11705

leandroalonso commented May 15, 2019

peril-wordpress-mobile bot commented May 15, 2019 •

edited

SylvesterWilmott commented May 17, 2019

SylvesterWilmott commented May 20, 2019

nheagy left a comment

nheagy May 22, 2019

leandroalonso May 23, 2019

nheagy May 22, 2019

leandroalonso May 24, 2019

nheagy May 22, 2019

leandroalonso May 23, 2019

nheagy May 22, 2019

leandroalonso May 23, 2019

nheagy May 22, 2019

leandroalonso May 23, 2019 •

edited

nheagy May 22, 2019

nheagy May 22, 2019

leandroalonso May 23, 2019

nheagy May 22, 2019

leandroalonso May 23, 2019

nheagy May 27, 2019

leandroalonso May 28, 2019

nheagy May 22, 2019

leandroalonso May 23, 2019

nheagy left a comment

nheagy May 27, 2019

leandroalonso May 28, 2019

nheagy left a comment

Update card design and do not display author name when "Me" is selected #11705

Update card design and do not display author name when "Me" is selected #11705

Conversation

leandroalonso commented May 15, 2019

peril-wordpress-mobile bot commented May 15, 2019 • edited

SylvesterWilmott commented May 17, 2019

SylvesterWilmott commented May 20, 2019

nheagy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leandroalonso May 23, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nheagy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nheagy left a comment

Choose a reason for hiding this comment

peril-wordpress-mobile bot commented May 15, 2019 •

edited

leandroalonso May 23, 2019 •

edited