Robert, I have a patch for this but am unsure if all of it makes sense and could use a code review.

The problem appears to be this: when the new composer is created, the "conversations-selected" signal in ConversationListView fires with an empty list.  That in turn is routed to the ConversationViewer, which goes into "zero items selected" mode, dropping the composer on the floor.

The easy fix was to simply not go into that mode if any inline composers are present, but there was a secondary problem: if the selected conversation was long and had scroll bars, the new composer matched its size.  (Also, if you'd scrolled down when you pressed new, the scroll bars remained in place.)  So, now if an inline composer is present, the viewer clears its view and moves the scroll bars to the top.  There's a little flashing, but it's not bad.

Can you code review?  Is there a better solution here?

Created attachment 295708 [details] [review]
Candidate fix.

I think the problem is that ComposerEmbed is monkeying around with the ConversationViewer when showing the INLINE_NEW composer.  First it causes the ConversationListView to switch to no selection, which causes the ConversationViewer to switch to displaying the "No messages selected" message.  But, for reasons I don't remember, we can't deal with the ConversationViewer in that state, so we call show_conversation_div(), which switches it back to conversation mode.

With the changes to the ListView, however, that first mode switch is put into an Idle handler, so it fires after we call show_conversation_div, which is causing these problems.  The patch you posted works for me, but I worry that it's just papering over the real problem, which is that these three objects are too intertwined.

I see three other ways we could go:
1) Put the show_conversation_div() call into its own Idle handler, so it fires later.  This strikes me as dangerous.
2) Adjust the CSS so that we can show in the inline composer without calling show_conversation_div().
3) Change the new composition state to use the paned composer, and hide the ConverationViewer when it's open.

I'm leaning towards (3), myself.  This strikes me as a good idea anyway, and it would allow us to cut a lot of code out of the embedded composer that manipulates the conversation view.  It does accelerate the need to make the paned composer not look crappy.

Created attachment 295727 [details] [review]
WIP -- Host new composers in ComposerBox, not in ComposerEmbed

Here's a start at option (3).  It works in brief testing.  There's still 
some code in ComposerEmbed (and our conversation viewer CSS) that 
handles the case where the embedded composer doesn't have a message to 
appear after.  That should only happen for INLINE_NEW messages, so that 
should never happen now.  We should definitely add a warning there.  I'm 
thinking we may also want to rip out that code to ensure that things 
break noticably if we ever find ourselves in that code path.

We defintely need some better styling for the paned composer.  It's bad 
when you have the conversation above, but it's even worse without it.  I 
think some padding around it in a darker color may be enough.

The ComposerBox now has the responsibility for restoring the 
conversation list selection.  I haven't tested what happens when it 
tries to restore a deleted conversation.  Whatever happens, it should be 
better than what the ComposerEmbed did.

This approach is fine, especially if it makes things less fragile between these objects.  The styling is a problem; I'd really like for this to look pretty much as it does now before committing.

I feel Geary is kinda broken with this bug.  I don't want to back out the selection fix, because I feel that's important to keep.  Do you know how long it would take for you to get this into shape?  I'm tempted to commit my workaround to get this problem solved now, and then commit your engineered solution later, when it's ready.

I think we can get something basic in pretty quickly.  I'm trying to match the CSS style of the embedded composer, but that's not really working.  I don't know if I'm doing it wrong or if GTK doesn't support the CSS stuff I'm trying to do.  But if I can't get that working this evening, I'll do a simple solution that can hold us for now.

Created attachment 295818 [details] [review]
Simulate the inline composer style for the paned composer

This almost replicates the inline style.  I haven't figured out how to 
do outset box-shadows with GTK yet.  It seems that many elements only 
support a portion of the CSS styles, but I haven't found a listing of 
which do which yet.

This is okay for the new composer, but I think it wastes too much space 
for the reply paned composer.  But the latter is currently looks bad in 
other ways, so I don't think this is worse.  Eventually, I think we want 
to restyle these composers, but this should work as a stopgap.

This patch is to be applied on top of the existing one, though we may 
want to smoosh it all together before committing.

Created attachment 295819 [details] [review]
Remove code handling INLINE_NEW composers

I assert that referred != null in the embed, because we should never be 
sent down that path.  I do allow for the possibility that the HTML 
element it should appear after can't be found.  I don't think this 
should ever happen, but I might be wrong.  So we have a warning there, 
but continue with our best effort anyway.

This can be squashed into the others before commit.

I've noticed this too.  I agree this should be fixed soon, one way or another!

Looks good!  Stylisticially this fine.

One issue, Robert: Rather than assert(referred != null), simply remove the nullable modifier from the argument, i.e. "Geary.Email referred".  Vala will generate code to soft assert if null is passed in.  It also will do some null checking where the argument is passed in.

Make that change and commit!

Squashed together and committed as 787e92e7.