Add popup menu to FrameTags in the Timeline

Add a new contextparams attribute to menu items in gui.xml that are feed
by the UI with params (e.g. the FrameTag popup needs to know what exact
tag we should edit, and it's a command Param provided by the Timeline).

data/gui.xml

@@ -668,6 +668,12 @@
         <param name="type" value="noop" />
       </item>
     </menu>
+
+    <menu id="frame_tag_popup">
+      <item command="FrameTagProperties" text="Tag &amp;Properties..." contextparams="true" />
+      <item command="RemoveFrameTag" text="&amp;Remove Tag" contextparams="true" />
+    </menu>
+
   </menus>
 
   <!-- tools -->

src/app/app_menus.cpp

@@ -87,6 +87,7 @@ void AppMenus::reload()
   m_framePopupMenu.reset(loadMenuById(handle, "frame_popup"));
   m_celPopupMenu.reset(loadMenuById(handle, "cel_popup"));
   m_celMovementPopupMenu.reset(loadMenuById(handle, "cel_movement_popup"));
+  m_frameTagPopupMenu.reset(loadMenuById(handle, "frame_tag_popup"));
 
   ////////////////////////////////////////
   // Load keyboard shortcuts for commands
@@ -214,6 +215,8 @@ Widget* AppMenus::convertXmlelemToMenuitem(TiXmlElement* elem)
     command_name ? CommandsModule::instance()->getCommandByName(command_name):
                    NULL;
 
+  bool contextparams = bool_attr_is_true(elem, "contextparams");
+
   // load params
   Params params;
   if (command) {
@@ -231,7 +234,7 @@ Widget* AppMenus::convertXmlelemToMenuitem(TiXmlElement* elem)
 
   // Create the item
   AppMenuItem* menuitem = new AppMenuItem(elem->Attribute("text"),
-                                          command, command ? &params: NULL);
+    command, (command && !contextparams ? &params: nullptr));
   if (!menuitem)
     return NULL;
 

src/app/app_menus.h

@@ -45,6 +45,7 @@ namespace app {
     Menu* getFramePopupMenu() { return m_framePopupMenu; }
     Menu* getCelPopupMenu() { return m_celPopupMenu; }
     Menu* getCelMovementPopupMenu() { return m_celMovementPopupMenu; }
+    Menu* getFrameTagPopupMenu() { return m_frameTagPopupMenu; }
 
     void applyShortcutToMenuitemsWithCommand(Command* command, Params* params, Key* key);
 
@@ -63,6 +64,7 @@ namespace app {
     base::UniquePtr<Menu> m_framePopupMenu;
     base::UniquePtr<Menu> m_celPopupMenu;
     base::UniquePtr<Menu> m_celMovementPopupMenu;
+    base::UniquePtr<Menu> m_frameTagPopupMenu;
     ScopedConnection m_recentFilesConn;
   };
 

src/app/commands/cmd_remove_frame_tag.cpp

@@ -12,8 +12,10 @@
 #include "app/app.h"
 #include "app/cmd/remove_frame_tag.h"
 #include "app/commands/command.h"
+#include "app/commands/params.h"
 #include "app/context.h"
 #include "app/context_access.h"
+#include "app/loop_tag.h"
 #include "app/transaction.h"
 #include "app/ui/main_window.h"
 #include "app/ui/timeline.h"
@@ -27,8 +29,12 @@ public:
   Command* clone() const override { return new RemoveFrameTagCommand(*this); }
 
 protected:
-  bool onEnabled(Context* context);
-  void onExecute(Context* context);
+  void onLoadParams(Params* params) override;
+  bool onEnabled(Context* context) override;
+  void onExecute(Context* context) override;
+
+private:
+  std::string m_tagName;
 };
 
 RemoveFrameTagCommand::RemoveFrameTagCommand()
@@ -38,6 +44,11 @@ RemoveFrameTagCommand::RemoveFrameTagCommand()
 {
 }
 
+void RemoveFrameTagCommand::onLoadParams(Params* params)
+{
+  m_tagName = params->get("name");
+}
+
 bool RemoveFrameTagCommand::onEnabled(Context* context)
 {
   return context->checkFlags(ContextFlags::ActiveDocumentIsWritable |
@@ -49,23 +60,22 @@ void RemoveFrameTagCommand::onExecute(Context* context)
   ContextWriter writer(context);
   Sprite* sprite(writer.sprite());
   frame_t frame = writer.frame();
+  FrameTag* foundTag = nullptr;
 
-  FrameTag* best = nullptr;
-  for (FrameTag* tag : sprite->frameTags()) {
-    if (frame >= tag->fromFrame() &&
-        frame <= tag->toFrame()) {
-      if (!best ||
-          (tag->toFrame() - tag->fromFrame()) < (best->toFrame() - best->fromFrame())) {
-        best = tag;
-      }
-    }
+  if (!m_tagName.empty())
+    foundTag = sprite->frameTags().getByName(m_tagName);
+
+  if (!foundTag) {
+    foundTag = get_shortest_tag(sprite, frame);
+    if (!foundTag)
+      return;
   }
 
-  if (!best)
+  if (!foundTag)
     return;
 
   Transaction transaction(writer.context(), "Remove Frame Tag");
-  transaction.execute(new cmd::RemoveFrameTag(sprite, best));
+  transaction.execute(new cmd::RemoveFrameTag(sprite, foundTag));
   transaction.commit();
 
   App::instance()->getMainWindow()->getTimeline()->invalidate();

src/app/ui/timeline.cpp

@@ -596,11 +596,23 @@ bool Timeline::onProcessMessage(Message* msg)
 
           case PART_FRAME_TAG:
             if (m_clk.frameTag) {
-              Command* command = CommandsModule::instance()
-                ->getCommandByName(CommandId::FrameTagProperties);
-              Params params;
-              params.set("name", m_clk.frameTag->name().c_str());
-              UIContext::instance()->executeCommand(command, &params);
+              if (mouseMsg->right()) {
+                Menu* popup_menu = AppMenus::instance()->getFrameTagPopupMenu();
+                if (popup_menu) {
+                  Params params;
+                  params.set("name", m_clk.frameTag->name().c_str());
+                  CommandsModule::instance()->getCommandByName(CommandId::FrameTagProperties)->loadParams(&params);
+                  CommandsModule::instance()->getCommandByName(CommandId::RemoveFrameTag)->loadParams(&params);
+                  popup_menu->showPopup(mouseMsg->position());
+                }
+              }
+              else if (mouseMsg->left()) {
+                Command* command = CommandsModule::instance()
+                  ->getCommandByName(CommandId::FrameTagProperties);
+                Params params;
+                params.set("name", m_clk.frameTag->name().c_str());
+                UIContext::instance()->executeCommand(command, &params);
+              }
             }
             break;
 

src/app/widget_loader.cpp

@@ -40,7 +40,6 @@ using namespace ui;
 using namespace app::skin;
 
 static int convert_align_value_to_flags(const char *value);
-static bool bool_attr_is_true(const TiXmlElement* elem, const char* attribute_name);
 static int int_attr(const TiXmlElement* elem, const char* attribute_name, int default_value);
 
 WidgetLoader::WidgetLoader()
@@ -632,13 +631,6 @@ static int convert_align_value_to_flags(const char *value)
   return flags;
 }
 
-static bool bool_attr_is_true(const TiXmlElement* elem, const char* attribute_name)
-{
-  const char* value = elem->Attribute(attribute_name);
-
-  return (value != NULL) && (strcmp(value, "true") == 0);
-}
-
 static int int_attr(const TiXmlElement* elem, const char* attribute_name, int default_value)
 {
   const char* value = elem->Attribute(attribute_name);

src/app/xml_document.cpp

@@ -45,4 +45,11 @@ void save_xml(XmlDocumentRef doc, const std::string& filename)
     throw XmlException(doc);
 }
 
+bool bool_attr_is_true(const TiXmlElement* elem, const char* attrName)
+{
+  const char* value = elem->Attribute(attrName);
+
+  return (value != NULL) && (strcmp(value, "true") == 0);
+}
+
 } // namespace app

src/app/xml_document.h

@@ -23,6 +23,8 @@ namespace app {
   XmlDocumentRef open_xml(const std::string& filename);
   void save_xml(XmlDocumentRef doc, const std::string& filename);
 
+  bool bool_attr_is_true(const TiXmlElement* elem, const char* attrName);
+
 } // namespace app
 
 #endif