Deleting contact shows blank right pane

[abbyad]

On 2660, deleting a contact showed a blank grey right pane. It should return to the place that the contact belonged to with a toast "Contact deleted".

[alxndrsn]

May be caused by recent work in either #1862 or #1348

[estellecomment]

Actually, should it always go to its parent place? If you were in the parent place just previously, then yes. But if you selected the contact from the left-hand list, then delete it, you'll jump to the parent place which was not the last page you were at (it was the contacts list)

Example : you are in contact list Then you select a family Then you delete it. I'd expect to go back to the contact list with no right pane. Instead, jumping to parent sends you to the parent contact :

Thoughts @abbyad ?

[alxndrsn]

IMO it should just go to No contact selected until we are properly tracking breadcrumbs for the HBB / chevron-back behaviour.

[estellecomment]

Stuff that should be fixed regardless of where to navigate back to :

• The "Contact Deleted" toast is missing, both on mobile and desktop.
• The desktop's right pane doesn't display "No contact selected" when you get to it after a deletion (it's empty)

[estellecomment]

The "No contact selected" is displayed when you're in contacts.detail state. It's not being displayed after deletion because we're redirecting to contacts. In practice, the links all over the UI are to contacts.detail, never contacts, and it doesn't look like we use contacts at all (except if you navigate to <url>/contact, without trailing slash).

What makes the most sense to me is to make contacts a transient state that redirects to contacts.detail.

[estellecomment]

The cosmetic UX stuff is done, and I filed a separate issue for navigation (#1975).

@alxndrsn, can you review please?

[alxndrsn]

In practice, the links all over the UI are to contacts.detail, never contacts, and it doesn't look like we use contacts at all (except if you navigate to <url>/contact, without trailing slash).

If contacts.detail is loading ContactsContentCtrl when it isn't needed, it sounds to me like we should fix /contacts. Will review!

[alxndrsn]

Hmm this behaviour actually seems consistent with other controllers... wonder why we have all these dead states.

[estellecomment]

If contacts.detail is loading ContactsContentCtrl when it isn't needed

I think it is needed : it loads the contact list in the left pane. That's needed for all child states of /contacts, so it should stick around as a parent state.

[alxndrsn]

If contacts.detail is loading ContactsContentCtrl when it isn't needed

I think it is needed : it loads the contact list in the left pane. That's needed for all child states of /contacts, so it should stick around as a parent state.

I'd have thought ContactsCtrl loaded the list and was parent state to both of ContactsContentCtrl, ContactsEditCtrl and ContactsReportCtrl...

[alxndrsn]

Very minor comments on commits.

[estellecomment]

Turns out what I'd done worked for contacts but not for tasks (??). What was missing was make the parent state abstract, so added that : https://github.com/angular-ui/ui-router/wiki/Frequently-Asked-Questions#how-to-set-up-a-defaultindex-child-state

Abstract states still have their controllers run, and have templates, like normal states. https://github.com/angular-ui/ui-router/wiki/Nested-States-&-Nested-Views#abstract-states

For messages I've tested by hand that it doesn't break the tour.

For reports, I have to figure out what the query param does and make sure the redirecting will not break it. There aren't any tests about that, so I don't want to touch it without writing tests. That'll be handed over to #1976.

[alxndrsn]

Causes #2006 & #2007 and potentially #2008. Recommend fixing states so that:

/contacts -> No contact selected /contacts/bad-id -> Contact not found

[estellecomment]

Removing that state-jumping stuff. Solves #2006 and #2007 on my local env. (#2008 is not very descriptive so don't know)

[garethbowen]

I removed the above commit as it caused the regression in #2043 . I'm not sure what state this issue is in now....

[estellecomment]

cursed? Back to zero.

[alxndrsn]

Not much point reviewing code that's been reverted.

[estellecomment]

Code looks good but it doesn't solve the issue on desktop, I still see a blank right pane after contact deletion.

[estellecomment]

Works for contacts, mobile and desktop.

For reports, on mobile and desktop, when you delete a report it stays displayed. The url does go to reports/. It does disappear from the reports list (on desktop), and apparently does get deleted (when you interact with it things go all wrong)

[estellecomment]

Actually, that's probably caused by #1963, clearSelected() is not called any more when id is empty.

https://github.com/medic/medic-webapp/commit/74d55eca393d2869de5266bfb47cd07032f0de23#diff-ae2ab03c7390b4a1da61f490703e73daR130

[estellecomment]

A couple questions : https://github.com/medic/medic-webapp/commit/1d27e50bb9fef67d23e9866a297f62892373d4cf

Contacts looks like it works fine. Reports has a problem : when you delete a report, it stays displayed in the LHS list.

[estellecomment]

Oh fun : navigation for messages deletion is now messed up. When you delete a message from a conversation, you land back on "No message selected" instead of staying on the conversation.

Delete the 2nd message :

You get sent back to "No message selected" :

[garethbowen]

LGTM

[estellecomment]

omg closed at last