Skip to content

Activity Log: adds pagination and fix NoResultsController#15274

Merged
leandroalonso merged 11 commits intodevelopfrom
task/paginate_activity_log
Nov 13, 2020
Merged

Activity Log: adds pagination and fix NoResultsController#15274
leandroalonso merged 11 commits intodevelopfrom
task/paginate_activity_log

Conversation

@leandroalonso
Copy link
Copy Markdown
Contributor

@leandroalonso leandroalonso commented Nov 11, 2020

Part of #15192

  • Fix the NoResultsController
  • Adds pagination to the Activity Log

To test

  1. Open the Activity Log
  2. make sure the "Loading Activities" message appears
  3. When the activities appears scroll down
  4. Make sure new activities are correctly loaded and in order (you can compare with web)

PR submission checklist:

  • I have considered adding unit tests where possible.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@leandroalonso leandroalonso added this to the 16.2 milestone Nov 11, 2020
@leandroalonso leandroalonso self-assigned this Nov 11, 2020
@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented Nov 11, 2020 •
edited
Loading

You can trigger an installable build for these changes by visiting CircleCI here.

@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented Nov 11, 2020 •
edited
Loading

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@leandroalonso leandroalonso mentioned this pull request Nov 11, 2020
Closed
13 tasks
emilylaguna
emilylaguna suggested changes Nov 11, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@emilylaguna emilylaguna left a comment

Choose a reason for hiding this comment

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

Awesome job!

During my testing on the iPhone 11 Pro running iOS 14.0 it looks like viewModel.loadMore() is being triggered every time I scroll and not just when I get to the bottom:

Screen Shot 2020-11-11 at 5 10 38 PM

@leandroalonso
Copy link
Copy Markdown
Contributor Author

leandroalonso commented Nov 12, 2020

@emilylaguna

is being triggered every time I scroll and not just when I get to the bottom:

This is expected. loadMore() is triggered while you scroll through the last "page" of the tableView, not when you reach the end. The reasons:

  1. If the user scrolls down enough, we'll load more items before the user reachs the end;
  2. The store already manages multiple concurrent requests, so I didn't add this state in the VC because it's already there (thus the warning you're seeing).

I don't believe that's an issue tho, but open to suggestions. :)

@emilylaguna
Copy link
Copy Markdown
Contributor

emilylaguna commented Nov 12, 2020

@leandroalonso makes sense. The number of log messages was concerning to me. What about disabling the log? or possibly moving the logic to scrollViewWillEndDragging?

@emilylaguna
Copy link
Copy Markdown
Contributor

emilylaguna commented Nov 12, 2020

I also see a number of new autolayout warnings when tapping on the Activity view:

	Probably at least one of the constraints in the following list is one you don't want. 
	Try this: 
		(1) look at each constraint and try to figure out which you don't expect; 
		(2) find the code that added the unwanted constraint or constraints and fix it. 
(
    "<NSLayoutConstraint:0x7fd8d5dfd000 UIView:0x7fd8d5d1bc80.height == 100   (active)>",
    "<NSLayoutConstraint:0x7fd8d5d34a10 UIView:0x7fd8d5d1bc80.width == UIView:0x7fd8d5d1bc80.height   (active)>",
    "<NSLayoutConstraint:0x7fd8772748b0 H:|-(30)-[UIStackView:0x7fd8772d54e0]   (active, names: '|':UIView:0x7fd87727abd0 )>",
    "<NSLayoutConstraint:0x7fd87726dcc0 H:[UIStackView:0x7fd8772d54e0]-(30)-|   (active, names: '|':UIView:0x7fd87727abd0 )>",
    "<NSLayoutConstraint:0x7fd877220100 UIView:0x7fd87727abd0.leading == UILayoutGuide:0x7fd87728cfe0'UIViewSafeAreaLayoutGuide'.leading   (active)>",
    "<NSLayoutConstraint:0x7fd865f324f0 UILayoutGuide:0x7fd87728cfe0'UIViewSafeAreaLayoutGuide'.trailing == UIView:0x7fd87727abd0.trailing   (active)>",
    "<NSLayoutConstraint:0x7fd877167780 'UISV-canvas-connection' UIStackView:0x7fd8772ccfd0.leading == _UILayoutSpacer:0x7fd8e5eaf280'UISV-alignment-spanner'.leading   (active)>",
    "<NSLayoutConstraint:0x7fd87716e580 'UISV-canvas-connection' UIStackView:0x7fd8772ccfd0.centerX == UIView:0x7fd8d5d1bc80.centerX   (active)>",
    "<NSLayoutConstraint:0x7fd87719db10 'UISV-canvas-connection' UIStackView:0x7fd8772d54e0.leading == _UILayoutSpacer:0x7fd8771685e0'UISV-alignment-spanner'.leading   (active)>",
    "<NSLayoutConstraint:0x7fd87717fd20 'UISV-canvas-connection' UIStackView:0x7fd8772d54e0.centerX == UIStackView:0x7fd8772ccfd0.centerX   (active)>",
    "<NSLayoutConstraint:0x7fd877152810 'UISV-spanning-boundary' _UILayoutSpacer:0x7fd8e5eaf280'UISV-alignment-spanner'.leading <= UIView:0x7fd8d5d1bc80.leading   (active)>",
    "<NSLayoutConstraint:0x7fd877179f00 'UISV-spanning-boundary' _UILayoutSpacer:0x7fd8771685e0'UISV-alignment-spanner'.leading <= UIStackView:0x7fd8772ccfd0.leading   (active)>",
    "<NSLayoutConstraint:0x7fd8771865c0 'UIView-Encapsulated-Layout-Width' UIView:0x7fd87726ca30.width == 0   (active)>",
    "<NSLayoutConstraint:0x7fd877288240 'UIViewSafeAreaLayoutGuide-left' H:|-(0)-[UILayoutGuide:0x7fd87728cfe0'UIViewSafeAreaLayoutGuide'](LTR)   (active, names: '|':UIView:0x7fd87726ca30 )>",
    "<NSLayoutConstraint:0x7fd87727e350 'UIViewSafeAreaLayoutGuide-right' H:[UILayoutGuide:0x7fd87728cfe0'UIViewSafeAreaLayoutGuide']-(0)-|(LTR)   (active, names: '|':UIView:0x7fd87726ca30 )>"
)

@leandroalonso
Copy link
Copy Markdown
Contributor Author

leandroalonso commented Nov 12, 2020

@emilylaguna fixed both issues.

emilylaguna
emilylaguna approved these changes Nov 12, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@emilylaguna emilylaguna left a comment

Choose a reason for hiding this comment

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

Looks good to me! :shipit:

@leandroalonso leandroalonso merged commit 5d6cbc0 into develop Nov 13, 2020
@leandroalonso leandroalonso deleted the task/paginate_activity_log branch November 13, 2020 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@houndci-bot houndci-bot houndci-bot left review comments

@emilylaguna emilylaguna emilylaguna approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@leandroalonso leandroalonso

Labels

Activity Log Jetpack [Type] Task

Projects

None yet

Milestone

16.2

Development

Successfully merging this pull request may close these issues.

3 participants

@leandroalonso @emilylaguna @houndci-bot