-
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
Post Details Comments: add table header with comments count #17560
Conversation
You can trigger an installable build for these changes by visiting CircleCI here. |
Hey @dvdchr . If changes are needed that prevent approval, would you mind removing the 18.8 Milestone? Thank you!! |
You can trigger optional UI/connected tests for these changes by visiting CircleCI here. |
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.
Found a few visual issues, and nothing major. Approving since this is still WIP! 👍
- Somehow the header view is displayed with a gray background for me, but it is displayed correctly in your screenshot 🤔 .
- I just realized while checking the designs in Zeplin earlier, but I think the "View All Comments" button should have a lighter gray border instead of having the
.text
color... (maybe it should beseparator
?). Here's the design from Zeplin:
Hey @dvdchr .
Huh, that's not good. I'll take a look! Thanks!
Yea, I noted that when I added the button: I just haven't done it yet. |
Oh, got it! 👍 Sorry, that must've escaped me. |
Ref: #17511
This adds a custom table header view for the Comments table that displays the total comment count.
I also made a couple changes to the
BorderedButtonTableViewCell
:To test:
postDetailsComments
feature.XX Comments
header is displayed with the total number of comments.Regression Notes
Potential unintended areas of impact
N/A. WIP.
What I did to test those areas of impact (or what existing automated tests I relied on)
N/A. WIP.
What automated tests I added (or what prevented me from doing so)
N/A. WIP.
PR submission checklist:
RELEASE-NOTES.txt
if necessary.