Skip to content

Conversation

@bummytime
Copy link
Contributor

@bummytime bummytime commented Dec 20, 2018

This PR updates the FSM states on the notifications screen to now include emptyUnfiltered and emptyFiltered (just like Orders).

notes_state

Now you will see the empty placeholder displayed if a given filter does not have corresponding notes (along with a button to remove the filter). The existing "no notifications" placeholder overlay displays when the filter == .all and the results are empty.

Fixes #542

Testing

  1. Build and run the app. (Try to have only one kind of notification in your list)
  2. Open the notifications tab
  • Verify you see the notes
  1. Select a filter (either orders or reviews) where there are no notes
  • Verify you see the empty filter placeholder
  1. Tap the "Remove filter" button in the placeholder
  • Verify the filter is removed and you see all the notes again.
  1. Select a filter (either orders or reviews) where there ARE notes
  • Verify you see the notes
  1. Select the all filter and then remove (trash all the notes)
  • Verify you see "no notifications" placeholder

@jleandroperez would you mind doing a quick review?

@bummytime bummytime added the type: bug A confirmed bug. label Dec 20, 2018
@bummytime bummytime added this to the 0.14 milestone Dec 20, 2018
@bummytime bummytime self-assigned this Dec 20, 2018
Copy link
Contributor

@jleandroperez jleandroperez left a comment

Choose a reason for hiding this comment

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

Looks great!!

:shipit:

@jleandroperez
Copy link
Contributor

@bummytime sir, i'm hitting the button on your behalf, so that this one makes it in time for the code freeze!!

@jleandroperez jleandroperez merged commit cfe11f7 into develop Dec 21, 2018
@jleandroperez jleandroperez deleted the issue/542-notification-filter branch December 21, 2018 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: bug A confirmed bug.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants