Improve UX when opening file sequences

* Now we can select the specific files that are part of the sequence
* New checkbox do the same for all dropped files (fix #1284)

data/widgets/open_sequence.xml

@@ -0,0 +1,22 @@
+<!-- ASEPRITE -->
+<!-- Copyright (C) 2016 by David Capello -->
+<gui>
+<window text="Notice" id="open_sequence">
+  <vbox>
+    <label text="Do you want to load the following files as an animation?" />
+    <view expansive="true" id="view" minwidth="128" minheight="64">
+      <listbox id="files" multiselect="true" />
+    </view>
+    <separator horizontal="true" />
+    <check id="repeat" text="Do the same for other files" />
+    <hbox>
+      <boxfiller />
+      <hbox homogeneous="true">
+	<button id="agree" text="&amp;Agree" closewindow="true" minwidth="60" />
+	<button id="skip" text="&amp;Skip" closewindow="true" minwidth="60" magnet="true" />
+      </hbox>
+      <boxfiller />
+    </hbox>
+  </vbox>
+</window>
+</gui>

src/app/commands/cmd_open_file.cpp

@@ -34,8 +34,7 @@
 
 namespace app {
 
-class OpenFileJob : public Job, public IFileOpProgress
+class OpenFileJob : public Job, public IFileOpProgress {
-{
 public:
   OpenFileJob(FileOp* fop)
     : Job("Loading file")
@@ -79,6 +78,8 @@ OpenFileCommand::OpenFileCommand()
   : Command("OpenFile",
             "Open Sprite",
             CmdRecordableFlag)
+  , m_repeatCheckbox(false)
+  , m_seqDecision(SequenceDecision::Ask)
 {
 }
 
@@ -86,6 +87,7 @@ void OpenFileCommand::onLoadParams(const Params& params)
 {
   m_filename = params.get("filename");
   m_folder = params.get("folder"); // Initial folder
+  m_repeatCheckbox = (params.get("repeat_checkbox") == "true");
 }
 
 void OpenFileCommand::onExecute(Context* context)
@@ -108,9 +110,23 @@ void OpenFileCommand::onExecute(Context* context)
   }
 
   if (!m_filename.empty()) {
+    int flags = (m_repeatCheckbox ? FILE_LOAD_SEQUENCE_ASK_CHECKBOX: 0);
+
+    switch (m_seqDecision) {
+      case SequenceDecision::Ask:
+        flags |= FILE_LOAD_SEQUENCE_ASK;
+        break;
+      case SequenceDecision::Agree:
+        flags |= FILE_LOAD_SEQUENCE_YES;
+        break;
+      case SequenceDecision::Skip:
+        flags |= FILE_LOAD_SEQUENCE_NONE;
+        break;
+    }
+
     base::UniquePtr<FileOp> fop(
       FileOp::createLoadDocumentOperation(
-        context, m_filename.c_str(), FILE_LOAD_SEQUENCE_ASK));
+        context, m_filename.c_str(), flags));
     bool unrecent = false;
 
     if (fop) {
@@ -119,10 +135,20 @@ void OpenFileCommand::onExecute(Context* context)
         unrecent = true;
       }
       else {
-        if (fop->isSequence())
+        if (fop->isSequence()) {
+
+          if (fop->sequenceFlags() & FILE_LOAD_SEQUENCE_YES) {
+            m_seqDecision = SequenceDecision::Agree;
+          }
+          else if (fop->sequenceFlags() & FILE_LOAD_SEQUENCE_NONE) {
+            m_seqDecision = SequenceDecision::Skip;
+          }
+
           m_usedFiles = fop->filenames();
-        else
+        }
+        else {
           m_usedFiles.push_back(fop->filename());
+        }
 
         OpenFileJob task(fop);
         task.showProgressWindow();

src/app/commands/cmd_open_file.h

@@ -17,10 +17,24 @@ namespace app {
 
   class OpenFileCommand : public Command {
   public:
+    enum class SequenceDecision {
+      Ask, Agree, Skip,
+    };
+
     OpenFileCommand();
     Command* clone() const override { return new OpenFileCommand(*this); }
 
-    const std::vector<std::string>& usedFiles() const { return m_usedFiles; }
+    SequenceDecision sequenceDecision() const {
+      return m_seqDecision;
+    }
+
+    void setSequenceDecision(SequenceDecision seqDecision) {
+      m_seqDecision = seqDecision;
+    }
+
+    const std::vector<std::string>& usedFiles() const {
+      return m_usedFiles;
+    }
 
   protected:
     void onLoadParams(const Params& params) override;
@@ -29,7 +43,9 @@ namespace app {
   private:
     std::string m_filename;
     std::string m_folder;
+    bool m_repeatCheckbox;
     std::vector<std::string> m_usedFiles;
+    SequenceDecision m_seqDecision;
   };
 
 } // namespace app

src/app/file/file.cpp

@@ -32,6 +32,8 @@
 #include "render/render.h"
 #include "ui/alert.h"
 
+#include "open_sequence.xml.h"
+
 #include <cstring>
 #include <cstdarg>
 
@@ -149,6 +151,7 @@ FileOp* FileOp::createLoadDocumentOperation(Context* context, const char* filena
   if (fop->m_format->support(FILE_SUPPORT_SEQUENCES)) {
     /* prepare to load a sequence */
     fop->prepareForSequence();
+    fop->m_seq.flags = flags;
 
     /* per now, we want load just one file */
     fop->m_seq.filename_list.push_back(filename);
@@ -179,18 +182,57 @@ FileOp* FileOp::createLoadDocumentOperation(Context* context, const char* filena
         }
       }
 
-      /* TODO add a better dialog to edit file-names */
+      // TODO add a better dialog to edit file-names
-      if ((flags & FILE_LOAD_SEQUENCE_ASK) && context && context->isUIAvailable()) {
+      if ((flags & FILE_LOAD_SEQUENCE_ASK) &&
-        /* really want load all files? */
+          context &&
-        if ((fop->m_seq.filename_list.size() > 1) &&
+          context->isUIAvailable() &&
-            (ui::Alert::show("Notice"
+          fop->m_seq.filename_list.size() > 1) {
-              "<<Possible animation with:"
+        app::gen::OpenSequence window;
-              "<<%s, %s..."
+        window.repeat()->setVisible(flags & FILE_LOAD_SEQUENCE_ASK_CHECKBOX ? true: false);
-              "<<Do you want to load the sequence of bitmaps?"
-              "||&Agree||&Skip",
-              base::get_file_name(fop->m_seq.filename_list[0]).c_str(),
-              base::get_file_name(fop->m_seq.filename_list[1]).c_str()) != 1)) {
 
+        for (const auto& fn : fop->m_seq.filename_list) {
+          auto item = new ui::ListItem(base::get_file_name(fn));
+          item->setSelected(true);
+          window.files()->addChild(item);
+        }
+
+        window.files()->Change.connect(
+          [&window]{
+            window.agree()->setEnabled(
+              window.files()->getSelectedChild() != nullptr);
+          });
+
+        window.openWindowInForeground();
+
+        // If the user selected the "do the same for other files"
+        // checkbox, we've to save what the user want to do for the
+        // following files.
+        if (window.repeat()->isSelected()) {
+          if (window.closer() == window.agree())
+            fop->m_seq.flags = FILE_LOAD_SEQUENCE_YES;
+          else
+            fop->m_seq.flags = FILE_LOAD_SEQUENCE_NONE;
+        }
+
+        if (window.closer() == window.agree()) {
+          // If the user replies "Agree", we load the selected files.
+          std::vector<std::string> list;
+
+          auto it = window.files()->children().begin();
+          auto end = window.files()->children().end();
+          for (const auto& fn : fop->m_seq.filename_list) {
+            ASSERT(it != end);
+            if (it == end)
+              break;
+            if ((*it)->isSelected())
+              list.push_back(fn);
+            ++it;
+          }
+
+          ASSERT(!list.empty());
+          fop->m_seq.filename_list = list;
+        }
+        else {
           // If the user replies "Skip", we need just one file name
           // (the first one).
           if (fop->m_seq.filename_list.size() > 1) {
@@ -923,6 +965,7 @@ FileOp::FileOp(FileOpType type, Context* context)
   m_seq.frame = frame_t(0);
   m_seq.layer = nullptr;
   m_seq.last_cel = nullptr;
+  m_seq.flags = 0;
 }
 
 void FileOp::prepareForSequence()

src/app/file/file.h

@@ -20,8 +20,9 @@
 
 #define FILE_LOAD_SEQUENCE_NONE         0x00000001
 #define FILE_LOAD_SEQUENCE_ASK          0x00000002
-#define FILE_LOAD_SEQUENCE_YES          0x00000004
+#define FILE_LOAD_SEQUENCE_ASK_CHECKBOX 0x00000004
-#define FILE_LOAD_ONE_FRAME             0x00000008
+#define FILE_LOAD_SEQUENCE_YES          0x00000008
+#define FILE_LOAD_ONE_FRAME             0x00000010
 
 namespace doc {
   class Document;
@@ -50,14 +51,18 @@ namespace app {
     FileOpSave
   } FileOpType;
 
-  class IFileOpProgress
+  class IFileOpProgress {
-  {
   public:
     virtual ~IFileOpProgress() { }
     virtual void ackFileOpProgress(double progress) = 0;
   };
 
   // Structure to load & save files.
+  //
+  // TODO This class do to many things. There should be a previous
+  // instance (class) to verify what the user want to do with the
+  // sequence of files, and the result of that operation should be the
+  // input of this one.
   class FileOp {
   public:
     static FileOp* createLoadDocumentOperation(Context* context, const char* filename, int flags);
@@ -107,6 +112,9 @@ namespace app {
     void sequenceSetHasAlpha(bool hasAlpha) {
       m_seq.has_alpha = hasAlpha;
     }
+    int sequenceFlags() const {
+      return m_seq.flags;
+    }
 
     const std::string& error() const { return m_error; }
     void setError(const char *error, ...);
@@ -152,6 +160,8 @@ namespace app {
       LayerImage* layer;
       Cel* last_cel;
       base::SharedPtr<FormatOptions> format_options;
+      // Flags after the user choose what to do with the sequence.
+      int flags;
     } m_seq;
 
     void prepareForSequence();

src/app/modules/gui.cpp

@@ -348,6 +348,7 @@ bool CustomizedGuiManager::onProcessMessage(Message* msg)
       {
         DropFilesMessage::Files files = static_cast<DropFilesMessage*>(msg)->files();
         UIContext* ctx = UIContext::instance();
+        OpenFileCommand cmd;
 
         while (!files.empty()) {
           auto fn = files.front();
@@ -365,9 +366,9 @@ bool CustomizedGuiManager::onProcessMessage(Message* msg)
           }
           // Load the file
           else {
-            OpenFileCommand cmd;
             Params params;
             params.set("filename", fn.c_str());
+            params.set("repeat_checkbox", "true");
             ctx->executeCommand(&cmd, params);
 
             // Remove all used file names from the "dropped files"

src/app/widget_loader.cpp

@@ -296,6 +296,10 @@ Widget* WidgetLoader::convertXmlElementToWidget(const TiXmlElement* elem, Widget
   else if (elem_name == "listbox") {
     if (!widget)
       widget = new ListBox();
+
+    bool multiselect = bool_attr_is_true(elem, "multiselect");
+    if (multiselect)
+      static_cast<ListBox*>(widget)->setMultiselect(multiselect);
   }
   else if (elem_name == "listitem") {
     ListItem* listitem;

src/ui/listbox.cpp

@@ -19,17 +19,27 @@
 #include "ui/theme.h"
 #include "ui/view.h"
 
+#include <algorithm>
+
 namespace ui {
 
 using namespace gfx;
 
 ListBox::ListBox()
   : Widget(kListBoxWidget)
+  , m_multiselect(false)
+  , m_firstSelectedIndex(-1)
+  , m_lastSelectedIndex(-1)
 {
   setFocusStop(true);
   initTheme();
 }
 
+void ListBox::setMultiselect(const bool multiselect)
+{
+  m_multiselect = multiselect;
+}
+
 Widget* ListBox::getSelectedChild()
 {
   for (auto child : children())
@@ -53,39 +63,87 @@ int ListBox::getSelectedIndex()
   return -1;
 }
 
-void ListBox::selectChild(Widget* item)
+int ListBox::getChildIndex(Widget* item)
 {
-  for (auto child : children()) {
+  const WidgetsList& children = this->children();
-    if (child->isSelected()) {
+  auto it = std::find(children.begin(), children.end(), item);
-      if (item && child == item)
+  if (it != children.end())
-        return;
+    return it - children.begin();
+  else
+    return -1;
+}
 
-      child->setSelected(false);
+Widget* ListBox::getChildByIndex(int index)
+{
+  const WidgetsList& children = this->children();
+  if (index >= 0 && index < int(children.size()))
+    return children[index];
+  else
+    return nullptr;
+}
+
+void ListBox::selectChild(Widget* item, Message* msg)
+{
+  int itemIndex = getChildIndex(item);
+  m_lastSelectedIndex = itemIndex;
+
+  if (m_multiselect) {
+    // Save current state of all children when we start selecting
+    if (msg == nullptr ||
+        msg->type() == kMouseDownMessage ||
+        msg->type() == kKeyDownMessage) {
+      m_firstSelectedIndex = itemIndex;
+      m_states.resize(children().size());
+
+      int i = 0;
+      for (auto child : children()) {
+        bool state = child->isSelected();
+        if (msg && !msg->ctrlPressed() && !msg->cmdPressed())
+          state = false;
+        m_states[i] = state;
+        ++i;
+      }
     }
   }
 
-  if (item) {
+  int i = 0;
-    item->setSelected(true);
+  for (auto child : children()) {
-    makeChildVisible(item);
+    bool newState;
+
+    if (m_multiselect) {
+      newState = m_states[i];
+
+      if (i >= MIN(itemIndex, m_firstSelectedIndex) &&
+          i <= MAX(itemIndex, m_firstSelectedIndex)) {
+        newState = !newState;
+      }
+    }
+    else {
+      newState = (child == item);
+    }
+
+    if (child->isSelected() != newState)
+      child->setSelected(newState);
+
+    ++i;
   }
 
+  if (item)
+    makeChildVisible(item);
+
   onChange();
 }
 
-void ListBox::selectIndex(int index)
+void ListBox::selectIndex(int index, Message* msg)
 {
-  const WidgetsList& children = this->children();
+  Widget* child = getChildByIndex(index);
-  if (index < 0 || index >= (int)children.size())
+  if (child)
-    return;
+    selectChild(child, msg);
-
-  ListItem* child = static_cast<ListItem*>(children[index]);
-  ASSERT(child);
-  selectChild(child);
 }
 
-std::size_t ListBox::getItemsCount() const
+int ListBox::getItemsCount() const
 {
-  return children().size();
+  return int(children().size());
 }
 
 void ListBox::makeChildVisible(Widget* child)
@@ -152,21 +210,20 @@ bool ListBox::onProcessMessage(Message* msg)
     case kMouseMoveMessage:
       if (hasCapture()) {
         gfx::Point mousePos = static_cast<MouseMessage*>(msg)->position();
-        int select = getSelectedIndex();
         View* view = View::getView(this);
         bool pick_item = true;
 
-        if (view) {
+        if (view && m_lastSelectedIndex >= 0) {
           gfx::Rect vp = view->viewportBounds();
 
           if (mousePos.y < vp.y) {
             int num = MAX(1, (vp.y - mousePos.y) / 8);
-            selectIndex(select-num);
+            selectIndex(MID(0, m_lastSelectedIndex-num, getItemsCount()-1), msg);
             pick_item = false;
           }
           else if (mousePos.y >= vp.y + vp.h) {
             int num = MAX(1, (mousePos.y - (vp.y+vp.h-1)) / 8);
-            selectIndex(select+num);
+            selectIndex(MID(0, m_lastSelectedIndex+num, getItemsCount()-1), msg);
             pick_item = false;
           }
         }
@@ -181,10 +238,11 @@ bool ListBox::onProcessMessage(Message* msg)
             picked = pick(mousePos);
           }
 
-          /* if the picked widget is a child of the list, select it */
+          // If the picked widget is a child of the list, select it
           if (picked && hasChild(picked)) {
-            if (ListItem* pickedItem = dynamic_cast<ListItem*>(picked))
+            if (ListItem* pickedItem = dynamic_cast<ListItem*>(picked)) {
-              selectChild(pickedItem);
+              selectChild(pickedItem, msg);
+            }
           }
         }
 
@@ -270,7 +328,7 @@ bool ListBox::onProcessMessage(Message* msg)
             return Widget::onProcessMessage(msg);
         }
 
-        selectIndex(MID(0, select, bottom));
+        selectIndex(MID(0, select, bottom), msg);
         return true;
       }
       break;

src/ui/listbox.h

@@ -11,6 +11,8 @@
 #include "obs/signal.h"
 #include "ui/widget.h"
 
+#include <vector>
+
 namespace ui {
 
   class ListItem;
@@ -19,13 +21,16 @@ namespace ui {
   public:
     ListBox();
 
+    bool isMultiselect() const { return m_multiselect; }
+    void setMultiselect(const bool multiselect);
+
     Widget* getSelectedChild();
     int getSelectedIndex();
 
-    void selectChild(Widget* item);
+    void selectChild(Widget* item, Message* msg = nullptr);
-    void selectIndex(int index);
+    void selectIndex(int index, Message* msg = nullptr);
 
-    std::size_t getItemsCount() const;
+    int getItemsCount() const;
 
     void makeChildVisible(Widget* item);
     void centerScroll();
@@ -41,6 +46,24 @@ namespace ui {
     virtual void onSizeHint(SizeHintEvent& ev) override;
     virtual void onChange();
     virtual void onDoubleClickItem();
+
+    int getChildIndex(Widget* item);
+    Widget* getChildByIndex(int index);
+
+    // True if this listbox accepts selecting multiple items at the
+    // same time.
+    bool m_multiselect;
+
+    // Range of items selected when we click down/up. Used to specify
+    // the range of selected items in a multiselect operation.
+    int m_firstSelectedIndex;
+    int m_lastSelectedIndex;
+
+    // Initial state (isSelected()) of each list item when the
+    // selection operation started. It's used to switch the state of
+    // items in case that the user is Ctrl+clicking items several
+    // items at the same time.
+    std::vector<bool> m_states;
   };
 
 } // namespace ui