-
Notifications
You must be signed in to change notification settings - Fork 47
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
Feature/new pages ui #247
Feature/new pages ui #247
Conversation
yanglin5689446
commented
Apr 13, 2020
•
edited
Loading
edited
- new article list UI
- new replies page UI
- search bar
- hoax for you page
- search result page(wip)
- paginator
@yanglin5689446 currently the file diff contains the changes from merged new-navbar-ui branch, which is pretty difficult to review. Would you rebase the commits? |
932c261
to
e026675
Compare
e026675
to
8c85e66
Compare
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.
First batch of comments
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.
Finished reviewing all files! It's great to see much progress on the list pages.
It is very challenging to design the props of ArticlePageLayout
. Let's move forward and see if we come up with better design in the future ;)
c2386af
to
8acbb0c
Compare
d232a1a
to
4fef5a8
Compare
isLink: false, | ||
showLastReply: true, | ||
showReplyCount: false, | ||
}} |
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.
Actually, replies page has different filter from article page.
Spec: https://g0v.hackmd.io/ZZWHWi2BTuyhkSWzAvKLAw#%E9%81%8E%E6%BF%BE%E9%81%B8%E9%A0%85%EF%BC%9A
The API for "我查核過" and reply type filtering is still on the way, but "還未有有效查核", "熱門回報" and "熱門討論" are legit ListArticle
filters already (hasPositiveFeedbackArticleReply
, replyRequestCount
and replyCount
respectively).
Considering the size of this change, I think we can handle this in future PRs, may need a ticket for this.
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.
card added here: https://github.com/orgs/cofacts/projects/5#card-37494391
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.
LGTM! Let's ship it to staging