WindowServer: Make automatic menu dismissal feel more "natural."

WindowServer/WSMenu.cpp

@@ -3,6 +3,7 @@
 #include "WSWindow.h"
 #include "WSMessage.h"
 #include "WSMessageLoop.h"
+#include "WSWindowManager.h"
 #include <SharedGraphics/Painter.h>
 #include <SharedGraphics/Font.h>
 
@@ -131,6 +132,7 @@ void WSMenu::did_activate(WSMenuItem& item)
 {
     if (on_item_activation)
         on_item_activation(item);
+    close();
 }
 
 WSMenuItem* WSMenu::item_at(const Point& position)
@@ -142,3 +144,8 @@ WSMenuItem* WSMenu::item_at(const Point& position)
     }
     return nullptr;
 }
+
+void WSMenu::close()
+{
+    WSWindowManager::the().close_menu(*this);
+};

WindowServer/WSMenu.h

@@ -62,6 +62,8 @@ public:
 
     Function<void(WSMenuItem&)> on_item_activation;
 
+    void close();
+
 private:
     void did_activate(WSMenuItem&);
 

WindowServer/WSWindowManager.cpp

@@ -199,7 +199,7 @@ WSWindowManager::WSWindowManager()
             menubar->add_menu(move(menu));
         }
         {
-            auto menu = make<WSMenu>("Application");
+            auto menu = make<WSMenu>("Terminal");
             menu->add_item(make<WSMenuItem>(5, "Foo."));
             menu->add_item(make<WSMenuItem>(6, "Bar?"));
             menu->add_item(make<WSMenuItem>(7, "Baz!"));
@@ -380,8 +380,9 @@ void WSWindowManager::notify_rect_changed(WSWindow& window, const Rect& old_rect
 
 void WSWindowManager::handle_menu_mouse_event(WSMenu& menu, WSMouseEvent& event)
 {
-    bool should_open_menu = (event.type() == WSMouseEvent::MouseMove && event.buttons() & (unsigned)MouseButton::Left)
+    bool is_hover_with_any_menu_open = event.type() == WSMouseEvent::MouseMove && m_current_menu;
-            || (event.type() == WSMouseEvent::MouseDown && event.button() == MouseButton::Left);
+    bool is_mousedown_with_left_button = event.type() == WSMouseEvent::MouseDown && event.button() == MouseButton::Left;
+    bool should_open_menu = &menu != m_current_menu && (is_hover_with_any_menu_open || is_mousedown_with_left_button);
 
     if (should_open_menu) {
         if (m_current_menu == &menu)
@@ -393,7 +394,7 @@ void WSWindowManager::handle_menu_mouse_event(WSMenu& menu, WSMouseEvent& event)
         m_current_menu = &menu;
         return;
     }
-    if (event.type() == WSMouseEvent::MouseUp && event.button() == MouseButton::Left) {
+    if (event.type() == WSMouseEvent::MouseDown && event.button() == MouseButton::Left) {
         close_current_menu();
         return;
     }
@@ -486,16 +487,16 @@ void WSWindowManager::process_mouse_event(WSMouseEvent& event)
         return;
     }
 
-    if (m_current_menu && m_current_menu->hovered_item() && !m_current_menu->menu_window()->rect().contains(event.position())) {
+    if (m_current_menu) {
-        m_current_menu->clear_hovered_item();
+        bool event_is_inside_current_menu = m_current_menu->menu_window()->rect().contains(event.position());
+        if (!event_is_inside_current_menu) {
+            if (m_current_menu->hovered_item())
+                m_current_menu->clear_hovered_item();
+            if (event.type() == WSMessage::MouseDown || event.type() == WSMessage::MouseUp)
+                close_current_menu();
+        }
     }
 
-    // FIXME: Figure out an automatic menu dismissal logic that feels right.
-#if 0
-    if (m_current_menu && event.type() == WSMouseEvent::MouseUp && event.button() == MouseButton::Left)
-        close_current_menu();
-#endif
-
     for (auto* window = m_windows_in_order.tail(); window; window = window->prev()) {
         if (!window->is_visible())
             continue;
@@ -764,3 +765,8 @@ void WSWindowManager::flush(const Rect& a_rect)
     }
 }
 
+void WSWindowManager::close_menu(WSMenu& menu)
+{
+    ASSERT(m_current_menu == &menu);
+    close_current_menu();
+}

WindowServer/WSWindowManager.h

@@ -51,6 +51,8 @@ public:
     Font& font() { return *m_font; }
     const Font& font() const { return *m_font; }
 
+    void close_menu(WSMenu&);
+
 private:
     WSWindowManager();
     virtual ~WSWindowManager() override;