Skip to content
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

Notifications List Mark I #402

Merged
merged 29 commits into from
Nov 6, 2018
Merged

Conversation

jleandroperez
Copy link
Collaborator

@jleandroperez jleandroperez commented Nov 5, 2018

Description:

This PR is a "Work in Progress":

  • Store-Y Notifications are (yet) not being filtered
  • The UI is not matching 100% the mockups. We'll address that in upcoming PR's!
  • Loading / Empty states not wired

Ref. #19

Details:

  • NotificationsViewController now displays stored Notifications
  • New Font: noticon
  • New Date Formatter: iso8601
  • New Notification Unit Test (timestamp as date parsing was failing!)
  • New NSAttributedString trimming method
  • Age Enum promoted to its own file

Testing:

  1. Log into a Dotcom Account
  2. Press over the Notifications tab
  3. Verify the Notifications list matches with what WPiOS renders

cc @bummytime @mindgraffiti
Thanks in advance!!

@wpmobilebot
Copy link
Collaborator

1 Warning
⚠️ PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 Danger

@jleandroperez jleandroperez requested review from mindgraffiti and bummytime and removed request for mindgraffiti November 5, 2018 19:53
@jleandroperez jleandroperez added feature: notifications Related to notifications or notifs. and removed [Status] Not Ready for Review labels Nov 5, 2018
@jleandroperez jleandroperez changed the title [WIP] Notifications List Mark I Notifications List Mark I Nov 5, 2018
@astralbodies astralbodies mentioned this pull request Nov 5, 2018
39 tasks
Copy link
Contributor

@mindgraffiti mindgraffiti left a comment

Choose a reason for hiding this comment

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

✅ UI sanity check
✅ code review

one quick question (unrelated to this PR)

:shipit:

@bummytime bummytime added this to MVLP Backlog in MVLP Kanban Board via automation Nov 6, 2018
@bummytime bummytime moved this from MVLP Backlog to Review/Testing in MVLP Kanban Board Nov 6, 2018
Copy link
Member

@bummytime bummytime left a comment

Choose a reason for hiding this comment

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

@jleandroperez I ❤️ all of this! SO great to finally see notes appearing in the app!

:shipit:

@jleandroperez
Copy link
Collaborator Author

Thank you both!!! 🔥

@jleandroperez jleandroperez merged commit 8263860 into develop Nov 6, 2018
MVLP Kanban Board automation moved this from Review/Testing to Done/Merged Nov 6, 2018
@jleandroperez jleandroperez deleted the issue/19-notifications-list branch November 6, 2018 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: notifications Related to notifications or notifs.
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants