-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Stats Refresh] Insights Comments: add detail list view #11181
[Stats Refresh] Insights Comments: add detail list view #11181
Conversation
… row is selected.
There was a problem hiding this comment.
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:
- If I tap a row, can the expected behavior open the site, or should the URL include the specific comment?
- 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!
Hey @stevebaranski ! Thanks for the review.
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).
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). |
@ScoutHarris sounds good! I'm happy to approve once he provides that clarity.
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.
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: |
Looks awesome @ScoutHarris !
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.
You're right @stevebaranski, 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. |
Thanks @stevebaranski ! |
Fixes #11160
On the Insights Comments card, selecting
View more
will now show a full list of Comments.View more
is selected, the filter tab (and thus the data) will be the tab selected on the overview card.No data yet
.NOTE: The UI for the details view still needs to be updated. That will be done with #11165.
To test: