From dca845d840b44a6155a55819ef58b85fc762e4ed Mon Sep 17 00:00:00 2001 From: Michael James Gratton Date: Mon, 22 Aug 2016 11:42:54 +1000 Subject: [PATCH] Reimplement in-conversation find. * src/client/application/geary-controller.vala (GearyController): Remove ACTION_FIND_NEXT_IN_CONVERSATION and ACTION_FIND_PREVIOUS_IN_CONVERSATION arctions and callbacks since they will be taken care of by the search entry & search bar buttons, and remove from accelerators.ui. Add ACTION_TOGGLE_FIND action to handle toggling find bar in the same way as the search bar. * src/client/components/main-toolbar.vala (MainToolbar): Add new button and infrastrcuture for toggling the find bar. * src/client/conversation-viewer/conversation-viewer.vala (ConversationViewer): Convert ::conversation_page to be grid, add new ::conversation_scroller property for the scrollbar, update call sites. Add props for accessing find widgets, remove old find methods and add callbacks for handling find start, change, etc. * src/client/conversation-viewer/conversation-email.vala, src/client/conversation-viewer/conversation-message.vala: Add methods for accessing selected text for find. * src/client/conversation-viewer/conversation-listbox.vala (ConversationListBox::highlight_search_terms): Updated to return a flag specifiying whether any search results were found, and to expand/collapse messsages depending on whether they have any. * src/client/conversation-viewer/conversation-message.vala (ConversationMessage::highlight_search_terms): Keep track of how many results were found, and return that. * ui/conversation-viewer.ui: Convert conversation_page to be a grid, add a search bar and search widgets to it, and move conversation ScrolledWindow to it. --- src/client/application/geary-controller.vala | 48 ++++---- src/client/components/main-toolbar.vala | 17 ++- src/client/components/main-window.vala | 2 + .../conversation-email.vala | 9 ++ .../conversation-listbox.vala | 24 +++- .../conversation-message.vala | 30 ++++- .../conversation-viewer.vala | 113 +++++++++++++----- ui/accelerators.ui | 2 - ui/conversation-viewer.ui | 102 +++++++++++++++- 9 files changed, 280 insertions(+), 67 deletions(-) diff --git a/src/client/application/geary-controller.vala b/src/client/application/geary-controller.vala index d013252d..db1dedd2 100644 --- a/src/client/application/geary-controller.vala +++ b/src/client/application/geary-controller.vala @@ -35,8 +35,6 @@ public class GearyController : Geary.BaseObject { public const string ACTION_EMPTY_TRASH = "GearyEmptyTrash"; public const string ACTION_UNDO = "GearyUndo"; public const string ACTION_FIND_IN_CONVERSATION = "GearyFindInConversation"; - public const string ACTION_FIND_NEXT_IN_CONVERSATION = "GearyFindNextInConversation"; - public const string ACTION_FIND_PREVIOUS_IN_CONVERSATION = "GearyFindPreviousInConversation"; public const string ACTION_ZOOM_IN = "GearyZoomIn"; public const string ACTION_ZOOM_OUT = "GearyZoomOut"; public const string ACTION_ZOOM_NORMAL = "GearyZoomNormal"; @@ -51,7 +49,8 @@ public class GearyController : Geary.BaseObject { public const string ACTION_SEARCH = "GearySearch"; public const string ACTION_CONVERSATION_LIST = "GearyConversationList"; public const string ACTION_TOGGLE_SEARCH = "GearyToggleSearch"; - + public const string ACTION_TOGGLE_FIND = "GearyToggleFind"; + public const string PROP_CURRENT_CONVERSATION ="current-conversations"; public const int MIN_CONVERSATION_COUNT = 50; @@ -474,15 +473,7 @@ public class GearyController : Geary.BaseObject { null, on_find_in_conversation_action }; entries += find_in_conversation; add_accelerator("slash", ACTION_FIND_IN_CONVERSATION); - - Gtk.ActionEntry find_next_in_conversation = { ACTION_FIND_NEXT_IN_CONVERSATION, null, null, - "G", null, on_find_next_in_conversation_action }; - entries += find_next_in_conversation; - - Gtk.ActionEntry find_previous_in_conversation = { ACTION_FIND_PREVIOUS_IN_CONVERSATION, - null, null, "G", null, on_find_previous_in_conversation_action }; - entries += find_previous_in_conversation; - + Gtk.ActionEntry archive_message = { ACTION_ARCHIVE_MESSAGE, ARCHIVE_MESSAGE_ICON_NAME, ARCHIVE_MESSAGE_LABEL, "A", null, on_archive_message }; archive_message.tooltip = ARCHIVE_MESSAGE_TOOLTIP_SINGLE; @@ -546,6 +537,11 @@ public class GearyController : Geary.BaseObject { _("Toggle search bar"), null }; entries += toggle_search; + // No callback is connected, since we bind the toggle button to the find bar visibility + Gtk.ActionEntry toggle_find = { ACTION_TOGGLE_FIND, null, null, null, + _("Toggle find bar"), null }; + entries += toggle_find; + return entries; } @@ -1298,6 +1294,9 @@ public class GearyController : Geary.BaseObject { private void on_folder_selected(Geary.Folder? folder) { debug("Folder %s selected", folder != null ? folder.to_string() : "(null)"); this.main_window.conversation_viewer.show_loading(); + GearyApplication.instance.get_action( + ACTION_FIND_IN_CONVERSATION + ).set_sensitive(false); enable_message_buttons(false); // If the folder is being unset, clear the message list and exit here. @@ -1507,6 +1506,9 @@ public class GearyController : Geary.BaseObject { private void on_conversations_selected(Gee.Set selected) { this.selected_conversations = selected; + GearyApplication.instance.get_action( + ACTION_FIND_IN_CONVERSATION + ).set_sensitive(false); ConversationViewer viewer = this.main_window.conversation_viewer; if (this.current_folder != null && !viewer.is_composer_visible) { switch(selected.size) { @@ -1527,6 +1529,9 @@ public class GearyController : Geary.BaseObject { try { viewer.load_conversation.end(ret); enable_message_buttons(!is_search); + GearyApplication.instance.get_action( + ACTION_FIND_IN_CONVERSATION + ).set_sensitive(true); } catch (Error err) { debug("Unable to load conversation: %s", err.message); @@ -2272,11 +2277,14 @@ public class GearyController : Geary.BaseObject { if (widget.state == ComposerWidget.ComposerState.NEW || widget.state == ComposerWidget.ComposerState.PANED) { main_window.conversation_viewer.do_compose(widget); + GearyApplication.instance.get_action( + ACTION_FIND_IN_CONVERSATION + ).set_sensitive(false); } else { ComposerEmbed embed = new ComposerEmbed( referred, widget, - main_window.conversation_viewer.conversation_page + main_window.conversation_viewer.conversation_scroller ); if (conversation_view != null) { conversation_view.add_embedded_composer(embed); @@ -2414,19 +2422,11 @@ public class GearyController : Geary.BaseObject { private void on_forward_message_action() { create_reply_forward_widget(ComposerWidget.ComposeType.FORWARD, null); } - + private void on_find_in_conversation_action() { - main_window.conversation_viewer.show_find_bar(); + this.main_window.conversation_viewer.conversation_find_bar.set_search_mode(true); } - - private void on_find_next_in_conversation_action() { - main_window.conversation_viewer.find(true); - } - - private void on_find_previous_in_conversation_action() { - main_window.conversation_viewer.find(false); - } - + private void on_archive_message() { archive_or_delete_selection_async.begin(true, false, cancellable_folder, on_archive_or_delete_selection_finished); diff --git a/src/client/components/main-toolbar.vala b/src/client/components/main-toolbar.vala index 78c0871e..5118f88c 100644 --- a/src/client/components/main-toolbar.vala +++ b/src/client/components/main-toolbar.vala @@ -14,6 +14,7 @@ public class MainToolbar : Gtk.Box { public bool show_close_button_left { get; private set; default = true; } public bool show_close_button_right { get; private set; default = true; } public bool search_open { get; set; default = false; } + public bool find_open { get; set; default = false; } public int left_pane_width { get; set; } private PillHeaderbar folder_header; @@ -106,17 +107,27 @@ public class MainToolbar : Gtk.Box { insert.add(conversation_header.create_popover_button("folder-symbolic", move_folder_menu, GearyController.ACTION_MOVE_MENU)); conversation_header.add_start(conversation_header.create_pill_buttons(insert)); - + + // Archive, undo, find insert.clear(); insert.add(archive_button = conversation_header.create_toolbar_button(null, GearyController.ACTION_ARCHIVE_MESSAGE, true)); insert.add(trash_delete_button = conversation_header.create_toolbar_button(null, GearyController.ACTION_TRASH_MESSAGE, false)); Gtk.Box archive_trash_delete = conversation_header.create_pill_buttons(insert); - + insert.clear(); insert.add(conversation_header.create_toolbar_button(null, GearyController.ACTION_UNDO, false)); Gtk.Box undo = conversation_header.create_pill_buttons(insert); - + + Gtk.Button find_button = folder_header.create_toggle_button( + "preferences-system-search-symbolic", GearyController.ACTION_TOGGLE_FIND); + this.bind_property("find-open", find_button, "active", + BindingFlags.SYNC_CREATE | BindingFlags.BIDIRECTIONAL); + insert.clear(); + insert.add(find_button); + Gtk.Box find = conversation_header.create_pill_buttons(insert); + + conversation_header.add_end(find); conversation_header.add_end(undo); conversation_header.add_end(archive_trash_delete); diff --git a/src/client/components/main-window.vala b/src/client/components/main-window.vala index 65bdac3a..e8e7c8ef 100644 --- a/src/client/components/main-window.vala +++ b/src/client/components/main-window.vala @@ -90,6 +90,8 @@ public class MainWindow : Gtk.ApplicationWindow { main_toolbar = new MainToolbar(); main_toolbar.bind_property("search-open", search_bar, "search-mode-enabled", BindingFlags.SYNC_CREATE | BindingFlags.BIDIRECTIONAL); + main_toolbar.bind_property("find-open", conversation_viewer.conversation_find_bar, "search-mode-enabled", + BindingFlags.SYNC_CREATE | BindingFlags.BIDIRECTIONAL); main_toolbar.show_close_button = true; set_titlebar(main_toolbar); diff --git a/src/client/conversation-viewer/conversation-email.vala b/src/client/conversation-viewer/conversation-email.vala index 60930a07..463a0756 100644 --- a/src/client/conversation-viewer/conversation-email.vala +++ b/src/client/conversation-viewer/conversation-email.vala @@ -550,6 +550,15 @@ public class ConversationEmail : Gtk.Box { : null; } + /** + * Returns user-selected body text from a message, if any. + */ + public string? get_selection_for_find() { + return (this.body_selection_message != null) + ? this.body_selection_message.get_selection_for_find() + : null; + } + /** * Attach an embedded composer to this email view. */ diff --git a/src/client/conversation-viewer/conversation-listbox.vala b/src/client/conversation-viewer/conversation-listbox.vala index de129c6b..14b80d05 100644 --- a/src/client/conversation-viewer/conversation-listbox.vala +++ b/src/client/conversation-viewer/conversation-listbox.vala @@ -456,8 +456,10 @@ public class ConversationListBox : Gtk.ListBox { /** * Applies search term highlighting to all email views. + * + * Returns true if any were found, else returns false. */ - public void highlight_search_terms(Gee.Set? search_matches) { + public bool highlight_search_terms(Gee.Set search_matches) { // Webkit's highlighting is ... weird. In order to actually // see all the highlighting you're applying, it seems // necessary to start with the shortest string and work up. @@ -467,10 +469,24 @@ public class ConversationListBox : Gtk.ListBox { ordered_matches.add_all(search_matches); ordered_matches.sort((a, b) => a.length - b.length); - message_view_iterator().foreach((msg_view) => { - msg_view.highlight_search_terms(search_matches); - return true; + bool any_found = false; + this.foreach((child) => { + EmailRow row = (EmailRow) child; + bool email_found = false; + row.view.message_view_iterator().foreach((msg_view) => { + if (msg_view.highlight_search_terms(search_matches) > 0) { + email_found = true; + } + return true; + }); + if (email_found) { + row.expand(); + any_found = true; + } else { + row.collapse(); + } }); + return any_found; } /** diff --git a/src/client/conversation-viewer/conversation-message.vala b/src/client/conversation-viewer/conversation-message.vala index b311d83d..9e847d84 100644 --- a/src/client/conversation-viewer/conversation-message.vala +++ b/src/client/conversation-viewer/conversation-message.vala @@ -465,8 +465,10 @@ public class ConversationMessage : Gtk.Grid { /** * Highlights user search terms in the message view. + & + * Returns the number of matching search terms. */ - public void highlight_search_terms(Gee.Set search_matches) { + public uint highlight_search_terms(Gee.Set search_matches) { // XXX Need to highlight subject, sender and recipient matches too // Remove existing highlights. @@ -481,11 +483,13 @@ public class ConversationMessage : Gtk.Grid { ordered_matches.add_all(search_matches); ordered_matches.sort((a, b) => a.length - b.length); + uint found = 0; foreach(string match in ordered_matches) { - web_view.mark_text_matches(match, false, 0); + found += web_view.mark_text_matches(match, false, 0); } web_view.set_highlight_text_matches(true); + return found; } /** @@ -538,6 +542,28 @@ public class ConversationMessage : Gtk.Grid { return quote; } + /** + * Returns the current selection as a string, suitable for find. + */ + internal string? get_selection_for_find() { + string? value = null; + WebKit.DOM.Document document = web_view.get_dom_document(); + WebKit.DOM.DOMWindow window = document.get_default_view(); + WebKit.DOM.DOMSelection selection = window.get_selection(); + + if (selection.get_range_count() > 0) { + try { + WebKit.DOM.Range range = selection.get_range_at(0); + value = range.get_text().strip(); + if (value.length <= 0) + value = null; + } catch (Error e) { + warning("Could not get selected text from web view: %s", e.message); + } + } + return value; + } + private SimpleAction add_action(string name, bool enabled, VariantType? type = null) { SimpleAction action = new SimpleAction(name, type); action.set_enabled(enabled); diff --git a/src/client/conversation-viewer/conversation-viewer.vala b/src/client/conversation-viewer/conversation-viewer.vala index 62a453b6..1ec614f8 100644 --- a/src/client/conversation-viewer/conversation-viewer.vala +++ b/src/client/conversation-viewer/conversation-viewer.vala @@ -53,7 +53,7 @@ public class ConversationViewer : Gtk.Stack { [GtkChild] private Gtk.Grid no_conversations_page; [GtkChild] - internal Gtk.ScrolledWindow conversation_page; + private Gtk.Grid conversation_page; [GtkChild] private Gtk.Grid multiple_conversations_page; [GtkChild] @@ -63,7 +63,20 @@ public class ConversationViewer : Gtk.Stack { [GtkChild] private Gtk.Grid composer_page; - private ConversationFindBar conversation_find_bar; + [GtkChild] + internal Gtk.ScrolledWindow conversation_scroller; + + [GtkChild] + internal Gtk.SearchBar conversation_find_bar; + + [GtkChild] + internal Gtk.SearchEntry conversation_find_entry; + + [GtkChild] + private Gtk.Button conversation_find_next; + + [GtkChild] + private Gtk.Button conversation_find_prev; // State machine setup for search/find modes. private Geary.State.MachineDescriptor search_machine_desc = new Geary.State.MachineDescriptor( @@ -131,6 +144,12 @@ public class ConversationViewer : Gtk.Stack { fsm = new Geary.State.Machine(search_machine_desc, mappings, null); fsm.set_logging(false); + this.conversation_find_bar.notify["search-mode-enabled"].connect( + on_find_search_started + ); + // XXX Do this in Glade when possible. + this.conversation_find_bar.connect_entry(this.conversation_find_entry); + //conversation_find_bar = new ConversationFindBar(web_view); //conversation_find_bar.no_show_all = true; //conversation_find_bar.close.connect(() => { fsm.issue(SearchEvent.CLOSE_FIND_BAR); }); @@ -158,25 +177,7 @@ public class ConversationViewer : Gtk.Stack { } }); this.composer_page.add(box); - set_visible_child(composer_page); - } - - /** - * Shows the in-conversation search UI. - */ - public void show_find_bar() { - fsm.issue(SearchEvent.OPEN_FIND_BAR); - conversation_find_bar.focus_entry(); - } - - /** - * Displays the next/previous match for an in-conversation search. - */ - public void find(bool forward) { - if (!conversation_find_bar.visible) - show_find_bar(); - - conversation_find_bar.find(forward); + set_visible_child(this.composer_page); } /** @@ -243,7 +244,7 @@ public class ConversationViewer : Gtk.Stack { new Geary.App.EmailStore(account), account.information, location.special_folder_type == Geary.SpecialFolderType.DRAFTS, - conversation_page.get_vadjustment() + conversation_scroller.get_vadjustment() ); // Need to fire this signal early so the the controller @@ -283,12 +284,12 @@ public class ConversationViewer : Gtk.Stack { viewport.show(); viewport.add(list); - this.conversation_page.add(viewport); + this.conversation_scroller.add(viewport); } // Remove any existing conversation list, cancelling its loading private void remove_current_list() { - Gtk.Widget? scrolled_child = this.conversation_page.get_child(); + Gtk.Widget? scrolled_child = this.conversation_scroller.get_child(); if (scrolled_child != null) { scrolled_child.destroy(); } @@ -314,10 +315,10 @@ public class ConversationViewer : Gtk.Stack { // Find bar opened. private uint on_open_find_bar(uint state, uint event, void *user, Object? object) { - if (!conversation_find_bar.visible) - conversation_find_bar.show(); + //if (!conversation_find_bar.visible) + // conversation_find_bar.show(); - conversation_find_bar.focus_entry(); + //conversation_find_bar.focus_entry(); //web_view.allow_collapsing(false); return SearchState.FIND; @@ -336,4 +337,62 @@ public class ConversationViewer : Gtk.Stack { // } } + private void on_find_search_started(Object obj, ParamSpec param) { + if (this.conversation_find_bar.get_search_mode()) { + if (this.current_list != null) { + ConversationEmail? email_view = + this.current_list.get_selection_view(); + if (email_view != null) { + string text = email_view.get_selection_for_find(); + if (text != null) { + this.conversation_find_entry.set_text(text); + this.conversation_find_entry.select_region(0, -1); + } + } + } + } + } + + [GtkCallback] + private void on_find_search_changed(Gtk.SearchEntry entry) { + string search = entry.get_text().strip(); + bool have_matches = false; + if (this.current_list != null) { + if (search.length > 0) { + // Have a search string + Gee.Set search_matches = new Gee.HashSet(); + search_matches.add(search); + have_matches = + this.current_list.highlight_search_terms(search_matches); + } else { + // Have no search string + // if (location is Geary.SearchFolder) { + // // Re-display the search results + // yield this.current_list.load_search_terms( + // (Geary.SearchFolder) location + // ); + // } else { + this.current_list.unmark_search_terms(); + // } + } + } + this.conversation_find_next.set_sensitive(have_matches); + this.conversation_find_prev.set_sensitive(have_matches); + } + + [GtkCallback] + private void on_find_next(Gtk.Widget entry) { + if (this.current_list != null) { + //this.current_list.show_prev_search_term(); + } + } + + [GtkCallback] + private void on_find_prev(Gtk.Widget entry) { + if (this.current_list != null) { + //this.current_list.show_next_search_term(); + } + } + } + diff --git a/ui/accelerators.ui b/ui/accelerators.ui index 549144a8..35c27f91 100644 --- a/ui/accelerators.ui +++ b/ui/accelerators.ui @@ -4,8 +4,6 @@ - - diff --git a/ui/conversation-viewer.ui b/ui/conversation-viewer.ui index 33579206..91b1e7fd 100644 --- a/ui/conversation-viewer.ui +++ b/ui/conversation-viewer.ui @@ -32,13 +32,105 @@ - + True - True - never - in + False - + + True + True + False + True + + + True + False + + + True + True + edit-find-symbolic + False + False + Find in conversation + + + + + + 0 + 0 + + + + + False + True + True + True + Find the previous occurrence of the search string. + + + + True + False + go-up-symbolic + + + + + 1 + 0 + + + + + False + True + True + True + Find the next occurrence of the search string. + + + + True + False + go-down-symbolic + + + + + 2 + 0 + + + + + + + + 0 + 0 + + + + + True + True + True + True + never + in + + + + + + 0 + 1 +