After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 713739 - compose new message in main window and inline replies
compose new message in main window and inline replies
Status: RESOLVED FIXED
Product: geary
Classification: Other
Component: composer
master
Other All
: High normal
: 0.8.0
Assigned To: Robert Schroll
Geary Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-01-04 12:39 UTC by Adam Dingle
Modified: 2014-05-21 00:44 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Test case for widget manipulation in plugins (2.01 KB, text/x-vala)
2013-05-28 04:44 UTC, Robert Schroll
Details

Description Charles Lindsay 2013-11-21 20:22:27 UTC


---- Reported by adam@yorba.org 2012-01-03 16:39:00 -0800 ----

Original Redmine bug id: 4554
Original URL: http://redmine.yorba.org/issues/4554
Searchable id: yorba-bug-4554
Original author: Adam Dingle
Original description:

Geary currently opens up a separate window for composing new messages. It
might be nice to allow the user to compose messages in the main Geary window
(presumably inline in any conversation in which the message is taking part).

Related issues:
related to geary - Feature #7653: Move Send button row to upper part of email
form (Open)
related to geary - Feature #6823: Allow the user to "pop out" composer window (Open)
related to geary - Feature #6902: Immediately display reply / forwarded
message in conversa... (Open)
related to geary - Feature #4322: in message composer, don't display lesser-
used fields by ... (Open)
related to geary - Bite-sized #5103: Closing composer window switches to
different application (Open)
related to geary - Feature #4606: display reply/reply-to-all buttons below
last message (Open)
related to geary - 6436: Composer toolbar dropdown menu with item to
toggle rich/p... (Fixed)
related to geary - Feature #6112: Consider using DecoratedWindow in Granite
for Compose Dialog (Open)
related to geary - Feature #5958: Inform user when replying to conversation
with multiple r... (Open)
duplicated by geary - 6793: reply to more specific message in
conversation (Duplicate)
duplicated by geary - Feature #5235: Inline/quick replies (Duplicate)
blocks geary - Feature #5957: Switch between Reply and Reply All (Open)



---- Additional Comments From geary-maint@gnome.bugs 2013-08-13 17:48:00 -0700 ----

### History

####

#1

Updated by Adam Dingle over 1 year ago

  * **Target version** deleted (<strike>_0.1_</strike>)

####

#2

Updated by Jim Nelson about 1 year ago

  * **Subject** changed from _compose messages in main window_ to _compose new message in main window and inline replies_
  * **Category** set to _composer_
  * **Target version** set to _0.3.0_

####

#3

Updated by Jim Nelson about 1 year ago

  * **Tracker** changed from _Bug_ to _Feature_

####

#4

Updated by Jim Nelson 10 months ago

  * **Target version** changed from _0.3.0_ to _0.4.0_

####

#5

Updated by Jim Nelson 8 months ago

  * **Target version** changed from _0.4.0_ to _0.5.0_

####

#6

Updated by Jim Nelson 7 months ago

Just to be clear what we're thinking for inline replies:

Our plan is that each message will have a Reply / Reply All / Forward links or
buttons along the bottom of each message that expands to show a composer.
Hitting the buttons in the toolbar are "conversation-based" replies. This
might mean jumping down to the bottom of the conversation with the cursor in
the composer widget, or going to the full-window composer prepped for reply.

Obviously there's more design discussion that needs to happen, but the upshot
is to rid Geary of the separate composer window.

####

#7

Updated by Robert Schroll 7 months ago

Jim Nelson wrote:

> but the upshot is to rid Geary of the separate composer window.

I'm not sure if you mean to get rid of the separate composer window being the
default, or to get rid of it completely. I'm okay with the first, but not the
second. Sometimes I want to refer to an email in another conversation while
I'm composing a message, or just look at a previous email in the current
conversation as I'm typing. Neither of these would work if the composer is
stuck in the conversation viewer.

####

#8

Updated by Eric Gregory 7 months ago

> Neither of these would work if the composer is stuck in the conversation
viewer.

If we had drafts you'd be able to switch back and forth without losing
anything, but I agree -- it would be better to allow the composer to "pop out"
into a new window, much like in Gmail.

####

#9

Updated by Jim Nelson 7 months ago

We've discussed having a "pop out" feature like Gmail (which I've used maybe
twice, but that's two more than zero) but we haven't ticketed it proper. Now
it is: #6823.

####

#10

Updated by Robert Schroll 6 months ago

  * **Assignee** set to _Robert Schroll_

####

#11

Updated by Robert Schroll 6 months ago

I have a basic version of this working. It's very much a work-in-progress, but
you can check it out if you're interested:
https://github.com/rschroll/geary/tree/inlinecomp. Basically, replies and
forwards get a composer inserted into the conversation view after the message
they're referring to. This composer can send an email, be closed, or pop out
into a separate window.

There are a number of problems still:

  * Some UI things don't update in the inline mode. I've noticed the toolbar when switching from rich to plain text and the list of attachments. I'm not sure why this is, but it may be related to how actions are (or aren't) set up in this mode.
  * Dialogs spawned by inline composers are attached to the main window. This is sort of odd, especially in Gnome Shell, where the dialogs are connected to the title bar. This could become confusing with multiple composers opened at once -- it wouldn't be clear who the dialog belongs to. One way to address this would be to use a Gtk.Overlay to place the dialogs on top of the composer within the WebView.
  * Instead of each inline composer asking if it should be closed, maybe they should all ask at once. Also, the inline composers should ask when you change to a different composition, and go away if they're dismissed. (Right now, they continue to hang around in the new conversation.
  * Compositions accelerators aren't hooked up in inline mode. I really don't know what we should do here.

Please let me know what you think.

####

#12

Updated by Jim Nelson 6 months ago

Robert, I get this error when I try to build this branch:

    
    
    make[3]: *** No rule to make target `/home/jim/webkit/lib/libwebkitgtk-3.0.so', needed by `src/geary'.  Stop.
    

####

#13

Updated by Robert Schroll 6 months ago

I accidentally left my 12.04 adjustments in the tree. If you revert 98a05d51,
it should work.

I'll try to figure out how I can publish the branch without this commit,
without jumping through hoops each time I push.

####

#14

Updated by Jim Nelson 6 months ago

This is definitely promising. I've noticed some UI glitches myself, which I'll
point out in a second just to be sure you're aware of them.

To answer / respond to your points:

  * How would Gtk.Overlay work here? I know little about the Overlay widget. Can an Overlay host/contain a Dialog?
  * I'm fine with changing the dismiss verification to a single dialog instead of one for each composer. Also, I'm fine with allowing only one inline composer to be open at a time, if that makes life easier. (i.e. if the user attempts to open another inline composer, a dialog prompts "Discard existing reply?")
  * We've been girding ourselves for the inevitable accelerator problems with this ticket. We've hit them already with Ctrl+F find and the upcoming Search box. There is some code in the client already to deal with the main window hosting an edit box where, when it has focus, the accelerators must be disabled. If you need more information, say so and I'll let Eric point you to the code you need to know about.

Some UI nits you may or may not know about:

  * One feature request is the ability to switch between Reply and Reply All (and Forward, although not as common) in #5957. I tried with the current branch. A new inlne composer was created each time I pressed the button. Worse, they weren't destroyed when I switched conversations. (I think this is what you were referring to in your comments.)
  * Attachments don't get listed when inline, but appear when popped out to a separate window.
  * The UI styling could be cleaned up. (I'm fine improving the styling in separate tickets.) In particular, I think a uniform (or more closely matched) font size would go a long way: #4508. I also think we should revisit the icon sizes in the composer toolbar. Again, not necessary to attack in this ticket.
  * The dream of inline replies is to make the reply look as much as the other messages in the conversation as possible. The _real_ dream is that when you press Send the composer switches to the sent message, a la #6902.

Great stuff! It looks like the surgery hasn't been awful yet. I do see
NP_Initialize calls showing up in the debug log; are those plugin calls?

####

#15

Updated by Robert Schroll 6 months ago

Jim Nelson wrote:

>   * How would Gtk.Overlay work here? I know little about the Overlay widget.
Can an Overlay host/contain a Dialog?

Dunno -- I've never used it before. The general plan would to make everything
the the composer insensitive and then put the dialog widgets over top of it. I
don't think we can just give it a Dialog, so I see two ways to deal with this:

1) Make ComposerContainers implement a file_dialog method, a color_dialog
method, etc. The ComposerWindow would just make those dialogs, while the
ComposerEmbed would create them in the overlay.

2) Give ComposerContainers a display_dialog method that accepts a dialog as an
argument. The ComposerWindow would just run the dialog, while the
ComposerEmbed would strip the contents out of the Dialog and put them into the
overlay. This would be simpler, but uglier.

One general problem I see is that the dialogs operate synchronously, but the
overlay would be much more natural in an asynchronous model. But see the
following point....

>   * I'm fine with changing the dismiss verification to a single dialog
instead of one for each composer. Also, I'm fine with allowing only one inline
composer to be open at a time, if that makes life easier. (i.e. if the user
attempts to open another inline composer, a dialog prompts "Discard existing
reply?")

I didn't even think of restricting it to one embedded composer. This would
require a bit more intelligence up front, but it would simplify a bunch of
other things. Notably, the issue of dialogs being attached to the main window
is less worrisome, since there won't be any confusion about which composer
they belong to.

One interesting implication of this would be that we could make a single
<embed> and a single ComposerEmbed right at the beginning. This could act as a
controller, figuring out what to do when we ask for a new composer. We only
have to worry about showing and hiding the <embed> appropriately, rather than
always creating and destroying the embeds.

>   * We've been girding ourselves for the inevitable accelerator problems
with this ticket. We've hit them already with Ctrl+F find and the upcoming
Search box. There is some code in the client already to deal with the main
window hosting an edit box where, when it has focus, the accelerators must be
disabled. If you need more information, say so and I'll let Eric point you to
the code you need to know about.

I can probably find it by reviewing how the search was done. But if Eric can
point the way, I'm sure it'd be helpful.

> Some UI nits you may or may not know about:

>

>   * One feature request is the ability to switch between Reply and Reply All
(and Forward, although not as common) in #5957. I tried with the current
branch. A new inlne composer was created each time I pressed the button.

Hmm. I had always pictured this being accomplished by an additional button
within the ComposerWidget. This is an interesting idea, but it would require
us to remember a bit more about the context of the current composition. One
immediate worry - what would happen if you pop the composer out into its own
window and then try to change from Reply to Reply All? Should it work or not?
And how could it work if you have multiple composition windows open?

> Worse, they weren't destroyed when I switched conversations. (I think this
is what you were referring to in your comments.)

This is a big problem, but I want to work out how to deal with dialogs in
general before applying that to this specific case.

>   * Attachments don't get listed when inline, but appear when popped out to
a separate window.

I have no idea why this is happening. My only guess is that some action isn't
hooked up properly, but the only thing different relative to the window case
is the accelerators. Could that be causing this?

>   * The UI styling could be cleaned up. (I'm fine improving the styling in
separate tickets.) In particular, I think a uniform (or more closely matched)
font size would go a long way: #4508. I also think we should revisit the icon
sizes in the composer toolbar. Again, not necessary to attack in this ticket.

Oh yeah. There's all sorts of ugliness going on here. But I want to make sure
this'll actually work before I try to make it pretty.

> Great stuff! It looks like the surgery hasn't been awful yet. I do see
NP_Initialize calls showing up in the debug log; are those plugin calls?

It hasn't been too bad, although I've had to do much more prepping here - I
have a little test program where I worked out all the the <embed> stuff. There
is a bunch of debug stuff that seems to be coming from the WebView when it
need to put in a plugin; the test program is also spitting that out. I don't
know what it means or how to disable it.

I'll start thinking about allowing only a single embedded composer. I suspect
this will end up saving us a bit of hassle.

Two other general things to think about:

1) Should new emails start embedded in the main window? This would require us
to undo the selection in the conversation list. What about clicks on email
addresses in a conversation? Would they open up as part of the conversation,
replace the conversation, or go into a new window?

2) Should we adjust the size of the <embed> to prevent the WebView in the
ComposerWidget from scrolling? This would allow us to avoid nested scrollbars,
which are annoying. But it also means the send button could be quite far away
when you're replying to a long email.

####

#16

Updated by Jim Nelson 6 months ago

Robert Schroll wrote:

> Hmm. I had always pictured this being accomplished by an additional button
within the ComposerWidget. This is an interesting idea, but it would require
us to remember a bit more about the context of the current composition. One
immediate worry - what would happen if you pop the composer out into its own
window and then try to change from Reply to Reply All? Should it work or not?
And how could it work if you have multiple composition windows open?

Good questions. I too envisioned having Reply/Reply All/Forward links or
buttons down near the embedded composer, but when I was trying out your patch
I realized that those buttons on the toolbar have to be dealt with as well. My
thinking is that those buttons only need to work for the embedded composer. If
it's been popped out to a separate window, then it should have its own
controls to deal with changing reply/reply all.

> >   * Attachments don't get listed when inline, but appear when popped out
to a separate window.

>

> I have no idea why this is happening. My only guess is that some action
isn't hooked up properly, but the only thing different relative to the window
case is the accelerators. Could that be causing this?

It's possible. When I was playing with it I was wondering if it was a
visibility problem (as in, the container holding the list of attachments was
hidden). It does look like it's a widget issue because when I popped out the
composer the attachment was ready to go and was sent.

> 1) Should new emails start embedded in the main window? This would require
us to undo the selection in the conversation list. What about clicks on email
addresses in a conversation? Would they open up as part of the conversation,
replace the conversation, or go into a new window?

I think the default mode should be the embedded composer, even for new
conversations. Perhaps we offer a Preference to change that, but our goal
since day one was to have embedded replies and composition. So, yes, this does
mean de-selecting the conversation list. Perhaps selecting the conversation
list is taken as a "discard" operation, meaning that if the user has started a
new message must confirm dismissing it before going to that conversation.

Clicks on email addresses should be like clicks in a separate browser
launching Geary: they start a new conversation, so the conversation viewer is
replaced.

I also think the "Discard" button in the embedded new-conversation composer
should act like a "Back" button. That may mean nothing more than selecting the
conversation being viewed when the New Message button was pressed. If the
conversation has been removed (or if none was selected to begin with), just
return to the "No Conversation Selected" state.

> 2) Should we adjust the size of the <embed> to prevent the WebView in the
ComposerWidget from scrolling? This would allow us to avoid nested scrollbars,
which are annoying. But it also means the send button could be quite far away
when you're replying to a long email.

I despise nested scrollbars, but we do live with them day-to-day with web
browsers. I'm tempted to suggested shortening the width of the composer so its
scroll bar is a fair distance from the main window's; that was messing me up
all the time with your branch.

For new conversations, I think we should be able to safely size the composer
so the only scroll bar is in the edit widget. That's good news. I think for
replies, we have to bite the bullet on this one; unless we were to auto-trim
quoted text (nope, we're not going to do that), my experience is that replies
to replies grow too fast to avoid this problem. As I said, maybe shortening
the width will ease the pain here.

As far as the send button, it's certainly worth considering if a layout change
is worth it. In Gmail, the embedded reply puts the Send/Discard buttons at the
top left of the "widget", near where the user is typing (if they're top-
replying). My instinct is that if we made such a change it should be reflected
in the pop-out composer too, though, but I'm amenable to that.

####

#17

Updated by Robert Schroll 6 months ago

I've pushed some new commits to the branch on github, if you'd care to check
them out. The modifications for 12.04 are still in there. I'm hoping it's easy
for you to merge with your modified version of this branch. But if this is
becoming a pain, let me know, and I'll revert it in the github branch.

New features:

  * Only a single inline composer is allowed at a time. If you do something that would open another, you get a dialog which allows you to open the new one in a new window. (I thought that was more sensible than moving the existing one to a new window and placing the new one inline.) You also get a warning if you try to switch away from a conversation with an inline composer. That one lets you move the composer to a new window. With only a single composer window inline, I think the standard dialogs will work just fine.
  * New messages are composed inline, with no conversation selected. Closing this, or popping it out to a new window, restores the previously selected conversation(s). This should work even if new emails arrive, but I haven't tested that part.
  * Accelerators work. Sorta. What I wanted to do was activate the composer accelerators whenever focus was within the ComposerEmbed. While GTK gives you signals about focus entering or leaving a widget, it doesn't seem to have anyway to notify you of focus being in any widget contained by a given widget. So I tried two things, and they're both in the tree, so you can see which you like best. 
    1. Only hook up the composer accelerators when focus is in the WebView. Most of the accelerators are for formatting options, so they will generally be used only when the focus is there. This is how it's set up right now.
    2. Hook up the composer accelerators whenever there's an inline composition. This means the composer accelerators will work whenever focus is anywhere in the composer, but they also work whenever focus is elsewhere in the main window. Since people will probably be focused on the composer when it's open, this may not be so bad. To see this behavior, checkout HEAD^ (6d836d62).  
The third option would be to hook up the focus signals for each an every
widget in the composer. This would achieve my original goal, but it would be a
pain.

_Edit (Hit submit instead of preview)_

More as a hold-over from the multiple-composer behavior than any conscious
decision, the inline composer immediately follows the message you're replying
to or forwarding. Perhaps it should instead appear only at the end of
conversations. This might simplify some code (we only have to show and hide
the embed, instead of adding and removing it), and it would put us in better
shape for the composer-morphs-into-new-message behavior.

This could also let us start playing around with tricks with the scrollbar.
It'd be less annoying to have the whole message at the bottom without a
scrollbar within the composer. (Though it'd still be somewhat annoying.) We
could also play tricks with the scrollbar in the conversation view. The top
portion of the scrollbar could move the whole conversation up and down, while
the bottom portion could scroll the composer webview. (This might be tricky to
set up, but I feel like it should be possible.) This lets us avoid dual
scrollbars without having super-long composers, but it does mean you have to
scroll to the top of the composition message before you can scroll back up in
the conversation. (But if you're referring back the the conversation often,
you'll probably pop the composer out into a separate window, right?)

Things that still need work:

  * The styling is a little bit better, but I make no attempt to match the appearance of the conversation view messages in the composer. We can do a little more polish here, but I'd prefer to put the matching styles work in another ticket.
  * Switching between reply and reply all. I'd prefer to leave that for a separate piece of work.
  * UI updates. I still don't know what's going on here, but I have two clues. 
    1. In the new composition mode, the composer is fit to the size of the window. If you add an attachment, it doesn't show up. But if you resize the window (and therefore the composer), the attachment suddenly appears. My guess is that a "hey - I got resized and need to be re-laid-out and re-drawn" signal is getting lost. But I don't know which signal that is.
    2. If you have a inline composer set to plain text that you then pop that into a new window, all of the toolitems that are supposed to be hidden suddenly appear! Again, I would suspect a missed signal.

Let me know what you think, and if you have any clues about the UI problems.

####

#18

Updated by Robert Schroll 6 months ago

  * **File** plugin2.vala added

Here's a little test case the illustrates the same problem we're having with
the UI updates. It makes a webview with an embed. The embedded widget has two
buttons. The top one inserts a label below itself. As you'll see, the label
doesn't appear until the window is resized. The bottom button will hide and
show the label below it. You'll see that the visibility of the label changes
immediately, but the space allocated to the label isn't removed/inserted until
the window is resized. I suspect the same thing is happening with the inserted
labels -- they're visible, but have no space to be visible in.

I don't think I'm doing anything wrong GTK-wise. If I insert the PluginWidget
directly into a Gtk.Window, I get everything behaving as I expect it to. Any
suggestions on where to go from here? Should I send this example to the
webkitgtk folk?

####

#19

Updated by Robert Schroll 6 months ago

I've sent a testcase to the webkitgtk mailing list, but no response yet.

Fooling around, I've found that I can get the testcase to redraw if I call on
the plugin widget `get_allocation(out alloc); size_allocate(alloc)`,
`resize_children()`, or `check_resize()`. The latter two are methods of
Gtk.Container that don't seem to be documented, so I don't know what their
intended use or side effects may be. Looking at the source, `resize_children`
just calls `get_allocation()`, `size_allocate()`, and `set_allocation()`.
`check_resize()` emits the 'check-resize' signal, which I'm guessing causes
the requisition/allocation cycle to run again.

I'll see about adding one of these methods to the ComposerWidget in
appropriate places and see if that clears up the UI issues. In the meantime,
I'm still curious if there are comments about anything else I talk about in
comment 17. Specifically: how to handle accelerators and where the composer
widget should go.

####

#20

Updated by Robert Schroll 6 months ago

I've updated the github branch. Recent changes have been merged in, and I
improved the UI updating using the allocation trick from the previous comment.
I'm occasionally getting warnings about trying to allocate widgets with
negative width; I don't know what that's about.

One oddity is that the quote level icons hang in the middle of the toolbar in
plain text mode, until you force another update of the UI. I suspect this is
related to the animation of the toolbar that we've noted before. It's probably
animating them sliding to the left, but the same no-updates bug is keeping
that from being noticed. Another reason to figure out how to get rid of the
animation.

####

#21

Updated by Jim Nelson 6 months ago

  * **Target version** changed from _0.5.0_ to _0.4.0_

Hi Robert, apologies for delay in response, I've been up to my neck in IMAP
rework.

I pulled the latest from github and tried it out. I also upgraded to Raring
today, which means that theming and such has changed out from under me. I
don't know if it's further work on your end or the theming changes, but this
latest version looks a lot cleaner than what I saw before. The inline reply
looks much more like an un-filled-out message in the conversation. That plus
instantly adding a send message to the conversation will really make this
feature shine, I'm certain of it now. I also think we should look at #4322
soon (by "we" meaning us, or you, or possibly Avi, who's doing some great work
for us now). That would further reduce clutter in the form. For that matter, I
think showing Subject: in an inline reply could be optional as well. Anything
we can do to make the 95% case a breeze for the times you want to just dash
out a reply. But that's for later.

As far as the redraw problems, that is certainly a common bane of GTK
programming. I know this is obvious, but have you tried simple queue_draw()
and/or queue_resize() calls? I don't know the WebKitGtk API very well, but I
wonder if there's a flag or option in there as well -- something that gives it
a clue that the widget needs redrawing/resizing.

As far as accelerators go, option #1 seems fine for me. Simpler the better,
unless there's some ungainly side-effect I'm missing. They work great for me
as-is. Is there a downside I'm unaware of?

I think I see the case for putting the reply at the bottom of the viewport no
matter what message is replied to. I think it would be better if replying to a
message in the middle of a conversation then collapsed all but the message you
were replying to, so that it's easy to consult while typing. The user could
open up other messages, if they liked. That does open up the reply-
becomes-a-message ticket. And anything we can do to avoid the scrollbar-
within-scrollbar is worth consideration. But are you suggesting stacked
scrollbars (one on top, one on the bottom)? I'm not keen on that, truth be
told.

One idea I've had is that for inline replies we simply hide the quoted text.
The user can open it up with a click (thinking big here, I realize this might
be difficult) for interspersing replies in quoted text. But in default mode,
which I suspect most people will simply accept (bottom-repliers excepted),
this gives a small non-scrolled window to type a reply, displaying To: Cc: (if
any), and the edit field. If they type a big message that is longer than the
box height, or open up the quoted text, then you get the scrollbars inside
scrollbars -- but like popping the message out of the window, that's more a
special case then the norm. Does that make sense?

More and more I'm leaning to the idea that inline reply should be clean,
small, quick, with a minimum of widgetry. Popped-out windows offer the full-
complement of controls and such, because real estate is suddenly cheaper and
the use suggests a more involved reply. Something like that.

But I think the work you're doing here is great and we're pretty excited to
see this land. This is a game-changing feature in my mind. Keep up the great
work!

####

#22

Updated by Robert Schroll 6 months ago

Jim Nelson wrote:

> I pulled the latest from github and tried it out. I also upgraded to Raring
today, which means that theming and such has changed out from under me. I
don't know if it's further work on your end or the theming changes, but this
latest version looks a lot cleaner than what I saw before.

I'll go ahead and take credit for everything. :)

> As far as the redraw problems, that is certainly a common bane of GTK
programming. I know this is obvious, but have you tried simple queue_draw()
and/or queue_resize() calls?

I think I tried this earlier, but I'll do it again to see if it works this
time.

> I don't know the WebKitGtk API very well, but I wonder if there's a flag or
option in there as well -- something that gives it a clue that the widget
needs redrawing/resizing.

No help from the webkitgtk list yet. I'll send an update with the clues about
allocation; that may trigger someone's memories.

> As far as accelerators go, option #1 seems fine for me. Simpler the better,
unless there's some ungainly side-effect I'm missing. They work great for me
as-is. Is there a downside I'm unaware of?

Just that the composer accelerators won't work when the focus is in the
address and subject entries. But since the accelerators are for formatting, I
don't think this is really too much of a problem. If we add a "Send"
accelerator, we might want to hook that up differently.

> I think I see the case for putting the reply at the bottom of the viewport
no matter what message is replied to. I think it would be better if replying
to a message in the middle of a conversation then collapsed all but the
message you were replying to, so that it's easy to consult while typing.

That's a neat idea. I like it. I'm guessing for search, we'll want to be able
to show certain messages and have the rest collapsed. So perhaps we should
come up with a general function to do that, which we could use here. (Unless
it's already done in the search branch?)

> But are you suggesting stacked scrollbars (one on top, one on the bottom)?
I'm not keen on that, truth be told.

My idea is to have a single scrollbar. Through most of its range, it scrolls
the whole conversation view. But the inline composer will come entirely into
view with some room left on the bottom of the scrollbar. Scrolling in that
region will leave the conversation view fixed, but scroll the inline composer
instead. This makes sense in my head, but it'd be a bit strange. I'd have to
see it actually working before I can decide if it's a good idea or not. (And
it'd probably take a bit of work to actually make work.)

> One idea I've had is that for inline replies we simply hide the quoted text.
The user can open it up with a click (thinking big here, I realize this might
be difficult) for interspersing replies in quoted text.

Top-posting is the devil! But I fear this battle is lost, so this sounds
reasonable. I'd put it in another ticket, though.

This coming week, I'll be traveling and at a conference, so I won't make much
progress. I intend to pick it back up afterwards, but if anyone wants to play
with this in the meantime, be my guest. If you do it on Github, just send me a
pull request and I'll merge it with my work. If you do it elsewhere, let me
know here.

####

#23

Updated by Robert Schroll 5 months ago

I've pushed a couple more commits to my branch, one of which puts the composer
at the bottom of the conversation. I include some code to hide all but the
message in question. Seeing it in action, I'm not so convinced this is the way
to go. My worries:

- If you reply to a message early in a conversation that still has unread messages, these messages will be marked as read when you get scrolled down to the composer. Moreover, these will get instantly collapsed (and maybe compressed (more reason to get rid of this!)), making it not at all clear which messages you've actually seen. (Of course, if you reply to a conversation without having read all the messages, maybe you deserve this!)  
- Even with hiding and compressing the messages, you generally don't get to see both the composer and the message in question at once. At the very least, this suggests that we shouldn't go through the trouble of changing the hidden state of messages, since it won't help. But if seeing the message being replied to is important, I think we need to have the composer right after that message.  
- What happens when a new message arrives in the conversation where you're composing? More to the point, what should happen? All solutions strike me as wrong: Putting it before the composer breaks the connection with the message being replied to. Putting it after the composer breaks consistency. Putting in somewhere in the hidden state is just plain weird.

That said, I recognize that putting the composer in the middle of
conversations will break the composed -> received transition that we're hoping
to make. Right now, I'm thinking that that's the lesser of two evils, but I'd
like to hear what everyone else thinks. (It's also worth remembering that
replying to intermediate messages in a conversation is probably not a
particularly common behavior, so we can live with some ugliness here.)

On a completely different note, sometimes the inline composer causes CPU usage
to spike. This seems to be deterministic, in that a given build will always
have this behavior. But the behavior will vary apparently randomly as I make
other changes to the build. Is anyone else seeing this?

####

#24

Updated by Jim Nelson 5 months ago

> - If you reply to a message early in a conversation that still has unread
messages, these messages will be marked as read when you get scrolled down to
the composer. Moreover, these will get instantly collapsed (and maybe
compressed (more reason to get rid of this!)), making it not at all clear
which messages you've actually seen. (Of course, if you reply to a
conversation without having read all the messages, maybe you deserve this!)

I know setting and clearing a lot of modal state in a GUI app is asking for
trouble, but could we put the conversation viewer in a state where it doesn't
mark messages as read when scrolling to the bottom, and then reset that state
when the composer is constructed and ready?

> - Even with hiding and compressing the messages, you generally don't get to
see both the composer and the message in question at once. At the very least,
this suggests that we shouldn't go through the trouble of changing the hidden
state of messages, since it won't help. But if seeing the message being
replied to is important, I think we need to have the composer right after that
message.

Hmmm ... that's a good point. Maybe a better way to put this is, if you use
the Reply/Reply All/Forward buttons in the toolbar, that places the composer
at the end of the conversation. If you reply/reply all/forward a message using
the message menu drop-down, you get the composer immediately after the
message. I don't expect the second case to be very common, though.

I see what you're saying. The composer really does need to be under the
message you're replying to. I'd hoped we could simplify the problem.

Again, I do hope that some of the clean-up work we've discussed for the
composer (reducing widget count) will go a long way toward keeping the
replied-to message visible in the viewport.

> - What happens when a new message arrives in the conversation where you're
composing? More to the point, what should happen? All solutions strike me as
wrong: Putting it before the composer breaks the connection with the message
being replied to. Putting it after the composer breaks consistency. Putting in
somewhere in the hidden state is just plain weird.

True. I know Gmail gets around this by popping up a little yellow window that
says "Load 1 new message(s)", or something like that. I used to think that was
a cop-out on their part, but over time I've come to see the wisdom in it. I
would not require this patch to include that to land this feature, but could
be something we work on immediately after.

> (It's also worth remembering that replying to intermediate messages in a
conversation is probably not a particularly common behavior, so we can live
with some ugliness here.)

Exactly.

> On a completely different note, sometimes the inline composer causes CPU
usage to spike. This seems to be deterministic, in that a given build will
always have this behavior. But the behavior will vary apparently randomly as I
make other changes to the build. Is anyone else seeing this?

I've been running your branch for a while now and haven't seen it. We have had
CPU spike problems in the past, and there's still potential for them depending
on how deep your mailbox is. Have you tried running from master? Do they only
appear with this branch?

Otherwise, I feel this is quite close and quite good. I can see some style
tweaking and other considerations we've discussed to fine-tune it, but I like
it.

Is there any possibility of moving the pop-out and close buttons onto the same
line as From:? That would save some real estate. Also, the "Do you want to
discard the existing composition?" warning appears even if I've not typed
anything. Kind of like the Discard button, I think that should only appear if
the user has made some changes.

How much more work do you think this needs to be ready?

####

#25

Updated by Robert Schroll 5 months ago

Jim Nelson wrote:

> I know setting and clearing a lot of modal state in a GUI app is asking for
trouble, but could we put the conversation viewer in a state where it doesn't
mark messages as read when scrolling to the bottom, and then reset that state
when the composer is constructed and ready?

Maybe. I don't actually know that the scroll to the bottom will affect the
read state of intermediate messages. But if the last message is visible above
the composer, that should be enough to mark it as read.

> Hmmm ... that's a good point. Maybe a better way to put this is, if you use
the Reply/Reply All/Forward buttons in the toolbar, that places the composer
at the end of the conversation. If you reply/reply all/forward a message using
the message menu drop-down, you get the composer immediately after the
message. I don't expect the second case to be very common, though.

>

> I see what you're saying. The composer really does need to be under the
message you're replying to. I'd hoped we could simplify the problem.

I think it needs to be right under that message **if** we want to be able to
see both the message and the composer. But I'm not convinced that this is
necessary. You have that message in the quoted text if you want to see it.

One idea that's been floating around in my head is to make a super-compressed
composer that's nothing more than a textbox, a send button, and an expand
button. When you reply to a specific message in a conversation, this would
open at the bottom of the message, letting you pen a simple reply. There'd be
no formatting options, no adjusting recipients (so this won't work for
forward; maybe we have a button to switch between reply and reply-all), no
changing the subject, and (perhaps) no editing the quoted text. Hitting the
expand button would tranfer you either to a full inline composer at the end of
the conversation or a popped out composer. This is so simple it could
conceivably be implemented in pure HTML, although for the sending
functionality it'd probably be better to make it a variant of the
ComposerWidget. The toolbar reply buttons could trigger the inline composer at
the end or just open this special mode for the last message. I don't know
which is better offhand.

> Again, I do hope that some of the clean-up work we've discussed for the
composer (reducing widget count) will go a long way toward keeping the
replied-to message visible in the viewport.

Maybe this compressed composer is just the end result of this plan....

> True. I know Gmail gets around this by popping up a little yellow window
that says "Load 1 new message(s)", or something like that. I used to think
that was a cop-out on their part, but over time I've come to see the wisdom in
it. I would not require this patch to include that to land this feature, but
could be something we work on immediately after.

I think we're okay here if we put the composer right after the message in
question all the time. Then, new messages will appear below the composer,
where the would have appeared otherwise, so everything is consistent.

> I've been running your branch for a while now and haven't seen it. We have
had CPU spike problems in the past, and there's still potential for them
depending on how deep your mailbox is. Have you tried running from master? Do
they only appear with this branch?

It only happens with the composer is inline, so I'm pretty sure it's my fault.
Popping the composer out resolves the problem, so it's very specific to in the
inline configuration. I've trapped it in gdb in the state, and it seems to be
spending its time doing allocation. This could be related to the issue of the
UI updates.

> Is there any possibility of moving the pop-out and close buttons onto the
same line as From:? That would save some real estate.

It's not so simple - those buttons live in the ComposerEmbed widget, which
holds the ComposerWidget in which the To line resides. I see two approaches
here. I could use a Gtk.Overlay to make them overlap, but that'd look bad
unless I can ensure that there's nothing under them. The other option is to
move them into the ComposerWidget, but only show them when it's embedded.
Right now, the ComposerWidget doesn't know whether it's in its own window or
not, but we may need to add that anyway to adjust the visibility of other
widgets to make the composer more compact when inline.

I think the second is the way to go, but maybe we can make it part of the more
general composer diet we're planning.

> Also, the "Do you want to discard the existing composition?" warning appears
even if I've not typed anything. Kind of like the Discard button, I think that
should only appear if the user has made some changes.

This was deliberate, not that that makes it correct. If you purposefully close
the inline composer, it only asks for confirmation if you've typed something.
But if you switch conversations or open another reply, it asks you regardless,
since I was afraid that the user might not realize that this meant the
composer would be closed. That said, I suppose users will get used to this
behavior rather quickly, so maybe this warning isn't necessary.

> How much more work do you think this needs to be ready?

In addition to figuring out where the composer should be located for
intermediate replies and slimming the composer down a bit, I'd like to
address:

- The UI update issue. The work-around does work, mostly, but there are still some problems. The indent buttons often hang in the middle of the toolbar when switching from HTML to plain text, and the composer scrollbar doesn't appear and disappear at appropriate times. (Updates can be triggered by scrolling the whole conversation view.) This may be related to the CPU issue. On the webkitgtk list, they recommend I submit a bug report. I'll try to get around to that soon.  
- When the composer is popped out, the To: entry is selected for some reason.  
- The tab key doesn't properly advance through the widgets in the inline mode.  
- Avoiding nested scrollbars. But I don't have a good idea here, so maybe that should be another bug.  
- Matching the styling of the inline composer with the other emails. This could be done in another bug as well.

For anyone that wants to play along, I've reverted my Ubuntu 12.04 patch in my
Github branch, so you should be able to clone it and build it without messing
about any. I've also merged with the trunk, so you don't have to give up
search to try this out.

####

#26

Updated by Robert Schroll 5 months ago

I finally got around to submitting a bug on the update issue to WebKit:
https://bugs.webkit.org/show_bug.cgi?id=118363

####

#27

Updated by Robert Schroll 5 months ago

I've solved the inline tabbing issue, although it's not particularly pretty.
I've pushed it to Github, and would like to hear if anyone has a better
solution.

####

#28

Updated by Jim Nelson 4 months ago

Been busy with a lot of other fires. Haven't forgotten about this, and
certainly want to see if we can get this in to 0.4.

Robert Schroll wrote:

> One idea that's been floating around in my head is to make a super-
compressed composer that's nothing more than a textbox, a send button, and an
expand button. When you reply to a specific message in a conversation, this
would open at the bottom of the message, letting you pen a simple reply.
There'd be no formatting options, no adjusting recipients (so this won't work
for forward; maybe we have a button to switch between reply and reply-all), no
changing the subject, and (perhaps) no editing the quoted text. Hitting the
expand button would tranfer you either to a full inline composer at the end of
the conversation or a popped out composer. This is so simple it could
conceivably be implemented in pure HTML, although for the sending
functionality it'd probably be better to make it a variant of the
ComposerWidget. The toolbar reply buttons could trigger the inline composer at
the end or just open this special mode for the last message. I don't know
which is better offhand.

I love this idea. I really think minimal is better for the inline reply. My
concern abour pure-HTML is that without Javascript (or perhaps some hooks into
the DOM, I don't know WebKitGTK well enough for sure), we lose a certain
amount of control and features. For example, while I like the idea of
minimizing the controls, I do think with the standard inline reply the user
should be able to add someone to the To: list (i.e. add them to the
conversation), and we should have the dropdown auto-complete for that.

My thinking is two-pronged: a minimal inline composer with some expander
arrows to open it up for lesser-used fields, and then the pop-out mode for the
full-banged display. I'm on the fence if the popped-out composer has more
available features than the inline. I would have to consider it on a case-by-
case basis, I guess. I don't think I would miss the editing toolbar in the
inline composer, although if we got the link button fixed, that _might_ be
handy for a certain class of user.

> I think we're okay here if we put the composer right after the message in
question all the time. Then, new messages will appear below the composer,
where the would have appeared otherwise, so everything is consistent.

Good point.

> It's not so simple - those buttons live in the ComposerEmbed widget, which
holds the ComposerWidget in which the To line resides. I see two approaches
here. I could use a Gtk.Overlay to make them overlap, but that'd look bad
unless I can ensure that there's nothing under them. The other option is to
move them into the ComposerWidget, but only show them when it's embedded.
Right now, the ComposerWidget doesn't know whether it's in its own window or
not, but we may need to add that anyway to adjust the visibility of other
widgets to make the composer more compact when inline.

Moving them into the ComposerWidget might be the ticket then. I wouldn't be
surprised if other features in the composer have to become visible/invisible
depending on its mode.

> In addition to figuring out where the composer should be located for
intermediate replies and slimming the composer down a bit, I'd like to
address:

- The UI update issue. The work-around does work, mostly, but there are still some problems. The indent buttons often hang in the middle of the toolbar when switching from HTML to plain text, and the composer scrollbar doesn't appear and disappear at appropriate times. (Updates can be triggered by scrolling the whole conversation view.) This may be related to the CPU issue. On the webkitgtk list, they recommend I submit a bug report. I'll try to get around to that soon.  
- When the composer is popped out, the To: entry is selected for some reason.  
- The tab key doesn't properly advance through the widgets in the inline mode.  
- Avoiding nested scrollbars. But I don't have a good idea here, so maybe that should be another bug.  
- Matching the styling of the inline composer with the other emails. This could be done in another bug as well.

I'd like to propose we move your branch into Yorba's as a feature branch. We
can then create sub-tickets for the above and patch the branch incrementally.
Some of the above might not make it for 0.4 -- nested scrollbars are something
I think we need to ponder, for example. Are you cool with that?

####

#29

Updated by Robert Schroll 4 months ago

Jim Nelson wrote:

> I'd like to propose we move your branch into Yorba's as a feature branch. We
can then create sub-tickets for the above and patch the branch incrementally.
Some of the above might not make it for 0.4 -- nested scrollbars are something
I think we need to ponder, for example. Are you cool with that?

Sounds good to me. I've just merged master into it, so it should be up to
date. I've also reverted the "composer-at-end" behavior in anticipation of the
minimal inline composers, although there's obviously plenty of work remaining
before we get there. If you'd like, I can rebase the branch onto the current
master, so it's a bit easier to see what's changed. (This would also let me
clean up the places where commits got reverted.)

####

#30

Updated by Jim Nelson 4 months ago

Robert Schroll wrote:

> If you'd like, I can rebase the branch onto the current master, so it's a
bit easier to see what's changed. (This would also let me clean up the places
where commits got reverted.)

That would help me out a lot. Thanks!

####

#31

Updated by Robert Schroll 4 months ago

Here you go: https://github.com/rschroll/geary/tree/inlinecomp2

I think I've cleaned everything up appropriately, but I haven't actually
checked that it will compile after each commit.

####

#32

Updated by Jim Nelson 3 months ago

  * **Target version** changed from _0.4.0_ to _0.5.0_



--- Bug imported by chaz@yorba.org 2013-11-21 20:22 UTC  ---

This bug was previously known as _bug_ 4554 at http://redmine.yorba.org/show_bug.cgi?id=4554
Imported an attachment (id=260775)

Unknown milestone "unknown in product geary. 
   Setting to default milestone for this product, "---".
Setting qa contact to the default for this product.
   This bug either had no qa contact or an invalid one.
Resolution set on an open status.
   Dropping resolution 

Comment 1 Robert Schroll 2014-02-14 07:31:32 UTC
I've finally bitten the bullet and gotten the branch updated to the current master.  You can see it here: https://github.com/rschroll/geary/tree/inlinecomp4

Unfortunately, changes since the last time I looked at this have change the destroy process for the composer, and I don't have it working yet.  Closing the composer when it's still inline results in a loop of calls to ComposerWidget.destroy().  No idea why; perhaps it makes sense to someone else.

Also, things will probably be very broken if any composers are open when you try to close Geary.
Comment 2 Jim Nelson 2014-02-19 20:01:59 UTC
This looks real good, Robert, save for the Close bug you mentioned.  When I tried to close Geary with the composer open, it simply didn't close.  But the functionality and the layout is there and I feel like this is close.

Unfortunately, we've passed feature freeze for 0.6 and I wouldn't feel comfortable adding this for the next release even if the above bugs were fixed.  What I would like to do is move this branch into our repo to make it easier to track master.  However, unless you become a member of the GNOME Foundation, you won't have commit rights, which means you need for us to commit branches.

I guess what I'm asking is, when would feel ready to move this into our repo?  I don't think we'll be able to commit serious coding time to help out for a few weeks.  But I would definitely like to have this ready for the next release, and the goal looks to be in sight.
Comment 3 Robert Schroll 2014-02-26 02:19:52 UTC
If you'd like to move this into your repository, go ahead.  I can continue working on github, and we can pull back and forth easily enough.  I think things are set up in a way that merging changes to the composer from master will no longer be such a pain.

However, I'm not entirely convinced the end is within sight.  There are still issues with getting everything updated correctly.  Scrollbars don't appear and disappear as they should.

An alternative approach would be to have an inline composer that's nothing more than a textarea or an editable iframe that would allow basic replies.  A button would then open up a full composer, either in a separate window or in the conversation pane (but as a GTK widget, not a WebKit plugin), where you could adjust the headers, do complex formatting, etc.
Comment 4 Yosef Or Boczko 2014-05-20 20:37:13 UTC
I just tried now the wip/713739-inline branch, and it look really nice.
It works to me when I try to replay to a message, but when I try to
write a new messgae I get a crash:
ERROR:/pkg/geary-git/src/geary/src/engine/api/geary-progress-monitor.vala:51:geary_progress_monitor_real_notify_start: assertion failed: (!is_in_progress)

Program received signal SIGABRT, Aborted.
0x00007ffff0e1ad67 in raise () from /usr/lib/libc.so.6
(gdb) bt
Python Exception <type 'exceptions.TypeError'> instance has no next() method: 
(gdb)
Comment 5 Jim Nelson 2014-05-20 21:11:33 UTC
Yosef, did you get a core dump when that happened?  A strack trace would be helpful.
Comment 6 Yosef Or Boczko 2014-05-20 21:13:11 UTC
Jim, all what I get I pasted here.
I don't know why, but I not get an empaty stack, so the `bt` not help here.
Comment 7 Yosef Or Boczko 2014-05-20 21:14:31 UTC
I see this too:
ERROR:/pkg/geary-git/src/geary/src/engine/api/geary-progress-monitor.vala:51:geary_progress_monitor_real_notify_start: assertion failed: (!is_in_progress)
Comment 8 Jim Nelson 2014-05-20 21:21:09 UTC
Bug #728936 reported the same problem, but that was fixed in master and has been merged into the inline branch (commit 8d407a7), so it must be a separate issue.

I don't believe this assertion is directly related to the inline composer.  If you see it again (especially, if you see it again while running Geary out of master), please file a ticket.  If a stack trace is available, that would certainly help.
Comment 9 Jim Nelson 2014-05-21 00:44:11 UTC
I'm pleased to announce that Robert Schroll's branch for the inline composer has been merged into Geary master.

The patch spans commit a1bf707 to commit 78d1d4.

Thanks, Robert!