Reminders to contacts - Githubissues

NataliaLKB commented 9 years ago

@benjaminlees do check why the tests are failing on Travis :). Will review the code locally soon.

benjaminlees commented 9 years ago

@NataliaLKB yeh silly mistake in the tests

NataliaLKB commented 9 years ago

@benjaminlees Great job with this!

Other comments I have are:

When I click on the bell on the contacts page I would expect the contact overlay to come up. Right now it is a link that doesn't seem to have a href.
The add a reminder button shouldn't show when your in view (not in edit) mode in contacts overlay. I also am not sure about the add reminder button actually. I would just start out with empty fields like in ledger.

But it seems to work great. I think Michael will be happy. :+1:

benjaminlees commented 9 years ago

@NataliaLKB thanks for your comments, super helpful!!!

The add button was added because unlike the ledger they need to be able to be empty when the rest of the contact is saved. For instance sometimes there must be no reminders or if a reminder is suddenly not needed it should be easy to remove. I also prefer the look of the ledger however I forsee a possible ux issue as the first reminder would have to be emptied.

There are however ways of getting round this and having just empty inputs when you start but they all require a fair bit of extra logic to ensure that the ux is still on par with what is provided by the button and I am unsure whether it is a priority at the moment.

NataliaLKB commented 9 years ago

@benjaminlees looking better!! :+1:

coot2coot / carrier-pigeon

Reminders to contacts #240