GNOME Bugzilla – Bug 713739
compose new message in main window and inline replies
Last modified: 2014-05-21 00:44:11 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
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.
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.
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.
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)
Yosef, did you get a core dump when that happened? A strack trace would be helpful.
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.
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)
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.
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!