F4 shows fg color popup again

beta11 removed the palette editor but we still needed a way to show/hide the color popup.

Fix #1527

data/gui.xml

@@ -114,11 +114,16 @@
       <key command="Timeline" shortcut="Tab">
         <param name="switch" value="true" />
       </key>
-      <key command="PaletteEditor" shortcut="A">
-        <param name="switch" value="true" />
-      </key>
+      <key command="PaletteEditor" shortcut="A" />
       <key command="PaletteEditor" shortcut="F4">
-        <param name="switch" value="true" />
+        <param name="edit" value="switch" />
+        <param name="popup" value="foreground" />
+      </key>
+      <key command="PaletteEditor">
+        <param name="popup" value="foreground" />
+      </key>
+      <key command="PaletteEditor">
+        <param name="popup" value="background" />
       </key>
       <key command="ShowExtras" shortcut="Ctrl+H" mac="Cmd+H" />
       <!-- Tabs -->
@@ -941,9 +946,7 @@
     </menu>
 
     <menu id="palette_popup">
-      <item command="PaletteEditor" text="Edit &amp;Palette">
-        <param name="switch" value="true" />
-      </item>
+      <item command="PaletteEditor" text="Edit &amp;Palette" />
       <item command="PaletteSize" text="Palette Si&amp;ze" />
       <separator />
       <item command="SetPaletteEntrySize" text="&amp;Small Size">

src/app/commands/cmd_palette_editor.cpp

@@ -23,11 +23,11 @@ protected:
   void onLoadParams(const Params& params) override;
   bool onChecked(Context* context) override;
   void onExecute(Context* context) override;
+  std::string onGetFriendlyName() const override;
 
 private:
-  bool m_open;
-  bool m_close;
-  bool m_switch;
+  bool m_edit;
+  bool m_popup;
   bool m_background;
 };
 
@@ -36,29 +36,19 @@ PaletteEditorCommand::PaletteEditorCommand()
             "Edit Palette",
             CmdRecordableFlag)
 {
-  m_open = true;
-  m_close = false;
-  m_switch = false;
+  m_edit = true;
+  m_popup = false;
   m_background = false;
 }
 
 void PaletteEditorCommand::onLoadParams(const Params& params)
 {
-  std::string target = params.get("target");
-  if (target == "foreground") m_background = false;
-  else if (target == "background") m_background = true;
-
-  std::string open_str = params.get("open");
-  if (open_str == "true") m_open = true;
-  else m_open = false;
-
-  std::string close_str = params.get("close");
-  if (close_str == "true") m_close = true;
-  else m_close = false;
-
-  std::string switch_str = params.get("switch");
-  if (switch_str == "true") m_switch = true;
-  else m_switch = false;
+  m_edit =
+    (params.empty() ||
+     params.get("edit") == "switch" ||
+     params.get("switch") == "true"); // "switch" for backward compatibility
+  m_popup = (!params.get("popup").empty());
+  m_background = (params.get("popup") == "background");
 }
 
 bool PaletteEditorCommand::onChecked(Context* context)
@@ -68,16 +58,56 @@ bool PaletteEditorCommand::onChecked(Context* context)
 
 void PaletteEditorCommand::onExecute(Context* context)
 {
-  bool state = ColorBar::instance()->inEditMode();
+  auto colorBar = ColorBar::instance();
+  bool editMode = colorBar->inEditMode();
+  ColorButton* button =
+    (m_background ?
+     colorBar->bgColorButton():
+     colorBar->fgColorButton());
 
-  if (m_switch)
-    state = !state;
-  else if (m_open)
-    state = true;
-  else if (m_close)
-    state = false;
+  // Switch edit mode
+  if (m_edit && !m_popup) {
+    colorBar->setEditMode(!editMode);
+  }
+  // Switch popup
+  else if (!m_edit && m_popup) {
+    if (button->isPopupVisible())
+      button->closePopup();
+    else
+      button->openPopup(true);
+  }
+  // Switch both
+  else if (m_edit && m_popup) {
+    if (editMode && button->isPopupVisible()) {
+      colorBar->setEditMode(false);
+      button->closePopup();
+    }
+    else {
+      if (!editMode)
+        colorBar->setEditMode(true);
+      if (!button->isPopupVisible())
+        button->openPopup(true);
+    }
+  }
+}
 
-  ColorBar::instance()->setEditMode(state);
+std::string PaletteEditorCommand::onGetFriendlyName() const
+{
+  std::string text = "Switch";
+  if (m_edit) {
+    text += " Edit Palette Mode";
+  }
+  if (m_edit && m_popup) {
+    text += " and";
+  }
+  if (m_popup) {
+    if (m_background)
+      text += " Background";
+    else
+      text += " Foreground";
+    text += " Color Popup";
+  }
+  return text;
 }
 
 Command* CommandFactory::createPaletteEditorCommand()

src/app/ui/color_bar.cpp

@@ -447,14 +447,9 @@ void ColorBar::onPaletteButtonClick()
 
   switch (static_cast<PalButton>(item)) {
 
-    case PalButton::EDIT: {
-      Command* cmd_show_palette_editor = CommandsModule::instance()->getCommandByName(CommandId::PaletteEditor);
-      Params params;
-      params.set("switch", "true");
-
-      UIContext::instance()->executeCommand(cmd_show_palette_editor, params);
+    case PalButton::EDIT:
+      setEditMode(!inEditMode());
       break;
-    }
 
     case PalButton::SORT: {
       gfx::Rect bounds = m_buttons.getItem(item)->bounds();
@@ -1206,11 +1201,9 @@ void ColorBar::updateCurrentSpritePalette(const char* operationName)
 
 void ColorBar::setupTooltips(TooltipManager* tooltipManager)
 {
-  Params params;
-  params.set("switch", "true");
   tooltipManager->addTooltipFor(
     m_buttons.getItem((int)PalButton::EDIT),
-    key_tooltip("Edit Color", CommandId::PaletteEditor, params), BOTTOM);
+    key_tooltip("Edit Color", CommandId::PaletteEditor), BOTTOM);
 
   tooltipManager->addTooltipFor(m_buttons.getItem((int)PalButton::SORT), "Sort & Gradients", BOTTOM);
   tooltipManager->addTooltipFor(m_buttons.getItem((int)PalButton::PRESETS), "Presets", BOTTOM);

src/app/ui/color_bar.h

@@ -79,6 +79,9 @@ namespace app {
     bool inEditMode() const;
     void setEditMode(bool state);
 
+    ColorButton* fgColorButton() { return &m_fgColor; }
+    ColorButton* bgColorButton() { return &m_bgColor; }
+
     // ContextObserver impl
     void onActiveSiteChange(const doc::Site& site) override;
 

src/app/ui/color_button.cpp

@@ -122,7 +122,7 @@ bool ColorButton::onProcessMessage(Message* msg)
     case kOpenMessage:
       if (!m_windowDefaultBounds.isEmpty() &&
           this->isVisible()) {
-        openSelectorDialog();
+        openPopup(false);
       }
       break;
 
@@ -250,13 +250,13 @@ void ColorButton::onClick(Event& ev)
   ButtonBase::onClick(ev);
 
   // If the popup window was not created or shown yet..
-  if (m_window == NULL || !m_window->isVisible()) {
+  if (!m_window || !m_window->isVisible()) {
     // Open it
-    openSelectorDialog();
+    openPopup(false);
   }
   else if (!m_window->isMoveable()) {
     // If it is visible, close it
-    closeSelectorDialog();
+    closePopup();
   }
 }
 
@@ -267,6 +267,8 @@ void ColorButton::onLoadLayout(ui::LoadLayoutEvent& ev)
     ev.stream() >> pinned;
     if (ev.stream() && pinned)
       ev.stream() >> m_windowDefaultBounds;
+
+    m_hiddenPopupBounds = m_windowDefaultBounds;
   }
 }
 
@@ -278,9 +280,15 @@ void ColorButton::onSaveLayout(ui::SaveLayoutEvent& ev)
     ev.stream() << 0;
 }
 
-void ColorButton::openSelectorDialog()
+bool ColorButton::isPopupVisible()
 {
-  bool pinned = (!m_windowDefaultBounds.isEmpty());
+  return (m_window && m_window->isVisible());
+}
+
+void ColorButton::openPopup(const bool forcePinned)
+{
+  bool pinned = forcePinned ||
+    (!m_windowDefaultBounds.isEmpty());
 
   if (m_window == NULL) {
     m_window = new ColorPopup(m_options);
@@ -293,8 +301,8 @@ void ColorButton::openSelectorDialog()
   m_window->setColor(m_color, ColorPopup::ChangeType);
   m_window->openWindow();
 
-  gfx::Rect winBounds = m_windowDefaultBounds;
-  if (!pinned) {
+  gfx::Rect winBounds;
+  if (!pinned || (forcePinned && m_hiddenPopupBounds.isEmpty())) {
     winBounds = gfx::Rect(m_window->bounds().origin(),
                           m_window->sizeHint());
     winBounds.x = MID(0, bounds().x, ui::display_w()-winBounds.w);
@@ -303,6 +311,12 @@ void ColorButton::openSelectorDialog()
     else
       winBounds.y = MAX(0, bounds().y-winBounds.h);
   }
+  else if (forcePinned) {
+    winBounds = m_hiddenPopupBounds;
+  }
+  else {
+    winBounds = m_windowDefaultBounds;
+  }
   winBounds.x = MID(0, winBounds.x, ui::display_w()-winBounds.w);
   winBounds.y = MID(0, winBounds.y, ui::display_h()-winBounds.h);
   m_window->setBounds(winBounds);
@@ -321,10 +335,12 @@ void ColorButton::openSelectorDialog()
   m_windowDefaultBounds = gfx::Rect();
 }
 
-void ColorButton::closeSelectorDialog()
+void ColorButton::closePopup()
 {
-  if (m_window != NULL)
-    m_window->closeWindow(NULL);
+  if (m_window) {
+    m_hiddenPopupBounds = m_window->bounds();
+    m_window->closeWindow(nullptr);
+  }
 }
 
 void ColorButton::onWindowColorChange(const app::Color& color)
@@ -347,7 +363,7 @@ void ColorButton::onActiveSiteChange(const Site& site)
     else {
       // Check if it's pinned from the preferences (m_windowDefaultBounds)
       if (!m_window && !m_windowDefaultBounds.isEmpty())
-        openSelectorDialog();
+        openPopup(false);
       // Or check if the window was hidden but it's pinned, so we've
       // to show it again.
       else if (m_window && m_window->isPinned())

src/app/ui/color_button.h

@@ -38,6 +38,10 @@ namespace app {
     app::Color getColor() const;
     void setColor(const app::Color& color);
 
+    bool isPopupVisible();
+    void openPopup(const bool forcePinned);
+    void closePopup();
+
     // IColorSource
     app::Color getColorByPosition(const gfx::Point& pos) override;
 
@@ -56,8 +60,6 @@ namespace app {
     void onSaveLayout(ui::SaveLayoutEvent& ev) override;
 
   private:
-    void openSelectorDialog();
-    void closeSelectorDialog();
     void onWindowColorChange(const app::Color& color);
     void onActiveSiteChange(const Site& site) override;
     bool canPin() const { return m_options.canPinSelector; }
@@ -66,6 +68,7 @@ namespace app {
     PixelFormat m_pixelFormat;
     ColorPopup* m_window;
     gfx::Rect m_windowDefaultBounds;
+    gfx::Rect m_hiddenPopupBounds;
     bool m_dependOnLayer;
     ColorButtonOptions m_options;
   };