Skip to content

[Stats Refresh] Insights Comments: add detail list view - #11181

Merged
ScoutHarris merged 5 commits into
developfrom
feature/11160-insights_comments_view_more
Mar 5, 2019
Merged

[Stats Refresh] Insights Comments: add detail list view#11181
ScoutHarris merged 5 commits into
developfrom
feature/11160-insights_comments_view_more

Conversation

@ScoutHarris

@ScoutHarris ScoutHarris commented Mar 4, 2019

Copy link
Copy Markdown
Contributor

Fixes #11160

On the Insights Comments card, selecting View more will now show a full list of Comments.

  • Currently, there is no API call to get the full list of Comments for either tab. So for now, this displays the data fetched from the "top" Comments API call used for the Insights Comments card. (cc @jklausa issue [Stats Refresh] Insights Comments detail list: correct data with new API #11182)
  • When View more is selected, the filter tab (and thus the data) will be the tab selected on the overview card.
  • Pull to refresh updates the data.
  • If one tab has data and the other does not, the empty tab will display No data yet.

NOTE: The UI for the details view still needs to be updated. That will be done with #11165.

To test:


  • On a site with more than 6 comments, go to Insights > Comments > View more.
  • Verify:
    • The tab titles match the overview card.
    • The selected tab match the overview card.

comments_list


  • On the Posts and Pages tab, selecting a row opens a web view for that post/page.

@ScoutHarris ScoutHarris added this to the 12.0 milestone Mar 4, 2019
@ScoutHarris ScoutHarris self-assigned this Mar 4, 2019
@ScoutHarris
ScoutHarris requested a review from a user March 4, 2019 19:11

@ghost ghost left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey @ScoutHarris - this is looking pretty good to me! I wanted to share my findings.

✅ Branch builds
✅ Tests pass
✅ Tapping View More behaves as expected, and honors the selected tab as you described.
✅ Tapping a row from Posts and Pages opens a web view

I have two questions:

  1. If I tap a row, can the expected behavior open the site, or should the URL include the specific comment?
  2. If Posts and Pages was selected when I navigate to Comments via View More, I'm observing that Authors is selected when I navigate back. Should that original selection be preserved?

Please let me know if you need additional information. Thanks!

@ScoutHarris

Copy link
Copy Markdown
Contributor Author

Hey @stevebaranski ! Thanks for the review.

If I tap a row, can the expected behavior open the site, or should the URL include the specific comment?

For Posts & Pages, the row selection is the same behavior as selecting a row from the Insights Comments card, which is it opens the post URL. If @SylvesterWilmott desires different behavior, it can be changed in a future PR (with some accompanying backend data alteration).

If Posts and Pages was selected when I navigate to Comments via View More, I'm observing that Authors is selected when I navigate back. Should that original selection be preserved?

The Android implementation does not persist the filter selection from details back to Insights, so I copied that behavior. However, if @SylvesterWilmott wishes it to be persisted, it can be done in a separate PR (as it should be the same for Insights Followers).

@ghost

ghost commented Mar 5, 2019

Copy link
Copy Markdown

@ScoutHarris sounds good! I'm happy to approve once he provides that clarity.

For Posts & Pages, the row selection is the same behavior as selecting a row from the Insights Comments card, which is it opens the post URL.

Acknowledged. I think this could be a nice enhancement, provided we have that information at our disposal. If we choose to create an issue for this, I agree that it should be considered in a future release.

The Android implementation does not persist the filter selection from details back to Insights, so I copied that behavior.

I'm happy to defer to Sylvester here. Personally I think this one warrants resolution prior to shipping, but I'm open to considering it subsequent to this PR. As it stands, the behavior is inconsistent:
• A → B → A
• A' → B' → A

@SylvesterWilmott

Copy link
Copy Markdown

Looks awesome @ScoutHarris !

If I tap a row, can the expected behavior open the site, or should the URL include the specific comment?

I'm a little confused, the rows don't refer to specific comments but instead only to that post and the number of comments it has.

If Posts and Pages was selected when I navigate to Comments via View More, I'm observing that Authors is selected when I navigate back. Should that original selection be preserved?

You're right @stevebaranski, we should preserve the tab selection. We can create an issue for this one but it isn't critical.

@ghost

ghost commented Mar 5, 2019

Copy link
Copy Markdown

we should preserve the tab selection. We can create an issue for this one but it isn't critical.

I have added this to the list of outstanding issues in #10840. Proceeding to approve.

@ScoutHarris

Copy link
Copy Markdown
Contributor Author

I have added this to the list of outstanding issues

Thanks @stevebaranski !

@ScoutHarris
ScoutHarris merged commit b572638 into develop Mar 5, 2019
@ScoutHarris
ScoutHarris deleted the feature/11160-insights_comments_view_more branch March 5, 2019 18:20
@ScoutHarris ScoutHarris mentioned this pull request Jun 4, 2019
79 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants