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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Post list revamp #12087

Merged
merged 184 commits into from Jul 11, 2019

Conversation

@nheagy
Copy link
Contributor

commented Jul 9, 2019

Fixes #11646

This PR will merge in all changes for the post list revamp 馃帀

To test:

  • Because this is the result of several previous PRs, you don't have to review the entire PR (that's the hope of breaking it up!) However, as a reviewer, you're welcome to comment on anything 馃榾
  • Ensure that the post list matches the mockups (see #11646)
  • Also ensure that any features added to the post list recently (last 1-2 months) are still present, due to this being a long-lived feature branch 馃榾

Note: This doesn't have the new colors, as they landed out-of-order. We should fix those in a separate PR.

Update release notes:

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

leandroalonso added some commits May 7, 2019

Creates a PostActionSheet to show post options
Given a view controller and a Post the Action Sheet with options is displayed
Adds an property to give "More" button a action callback
This new behavior coexists with the actual one. If no callback is provided the functionality remains as it was
Moves setActionSheetDelegate to PostCardTableViewCell header
Since this is a specific requirement for the cell itself, it should not be in InteractivePostView. If in the future this changes, we can move it to a proper protocol.
Calls the delegate passing a reference to the button
This way the action sheet appears correctly in iPad

@nheagy nheagy added this to the 12.9 milestone Jul 9, 2019

@nheagy nheagy self-assigned this Jul 9, 2019

@peril-wordpress-mobile

This comment has been minimized.

Copy link

commented Jul 9, 2019

Warnings
鈿狅笍 PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 馃毇 dangerJS

@nheagy nheagy requested a review from yaelirub Jul 9, 2019

@nheagy nheagy referenced this pull request Jul 9, 2019
30 of 38 tasks complete
@yaelirub

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2019

Hey @leandroalonso , @nheagy while testing this I noticed a few things:

  1. The 'expand cards' icon at the top right is not visible, but is somewhat tappable (though hard to tap)
    posts_list

  2. After taping "View" on a row in the list view and viewing the post, when I hit back, the table automatically scrolls up. I'm not sure if that's the expected experience (I hope not 馃槵 )
    posts_list2

@yaelirub
Copy link
Contributor

left a comment

The post list view icon is not visible. I also saw some weird behavior with deleting a post and the list in its different views but not sure if it's related to this PR

@leandroalonso

This comment has been minimized.

Copy link
Contributor

commented Jul 10, 2019

@yaelirub this definitely is not the expected experience. I will fix it 鈥 btw, the same problem is happening with the pages list, the fix will apply to both pages and posts.

leandroalonso and others added some commits Jul 10, 2019

Merge pull request #12093 from leandroalonso/issue/11646_post_list_re鈥
鈥amp_fixes

Issue/11646 post list revamp fixes
Remove reference to deleted file
- cause by merge problem
Fix test failing because of outdated pod
- broken due to merge

@nheagy nheagy requested a review from yaelirub Jul 10, 2019

@nheagy

This comment has been minimized.

Copy link
Contributor Author

commented Jul 10, 2019

@yaelirub I've updated the branch to fix the missing icons, and the failing test. Ready for another look!

@yaelirub
Copy link
Contributor

left a comment

I confirm that the missing icon and the table scrolling to the top are fixed.

I didn't go over everything but made sure the post list matches the mockups and that everything works as expected. I did notice a couple of layout issues in the post list and a few issues with "delete and undo" posts but not sure if they are related to this PR.

@nheagy nheagy merged commit 1eec9ed into develop Jul 11, 2019

3 checks passed

Hound No violations found. Woof!
Peril Found some issues. Don't worry, everything is fixable.
Details
ci/circleci: build_and_test Your tests passed on CircleCI!
Details

@nheagy nheagy deleted the issues/11646-post-list-revamp branch Jul 11, 2019

@nheagy

This comment has been minimized.

Copy link
Contributor Author

commented Jul 11, 2019

Thanks @yaelirub!

@yaelirub

This comment has been minimized.

Copy link
Contributor

commented Jul 12, 2019

@leandroalonso , @nheagy, I created this issue. The only one I could consistently reproduce.
#12113

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can鈥檛 perform that action at this time.