Arange horizontal/vertical now also sorts by filename

CHANGELOG.rst

@@ -13,8 +13,7 @@ Added
 * Added a confirmation dialog when attempting to close unsaved files.
   The confirmation dialog can be disalbed in:
   Settings -> Miscellaneous -> Confirm when closing an unsaved file
-* Add option to arrange by filename (Arrange -> By Filename)
-
+* Add option to arrange by filename (Arrange -> Square (by filename))
 
 Fixed
 -----
@@ -24,6 +23,12 @@ Fixed
 * Fixed a hang when saving an open bee file that has been removed
   since being opened
 
+Changed
+-------
+
+* Arrange Horiszontal/Vertical now also sort by filename instead of
+  the previous seemingly random behaviour
+
 
 0.3.3 - 2024-05-05
 ==================

beeref/actions/actions.py

@@ -250,20 +250,20 @@ def get_default_shortcut(self, index):
     ),
     Action(
         id='arrange_horizontal',
-        text='&Horizontal',
+        text='&Horizontal (by filename)',
         callback='on_action_arrange_horizontal',
         group='active_when_selection',
     ),
     Action(
         id='arrange_vertical',
-        text='&Vertical',
+        text='&Vertical (by filename)',
         callback='on_action_arrange_vertical',
         group='active_when_selection',
     ),
     Action(
-        id='arrange_by_filename',
-        text='By &Filename',
-        callback='on_action_arrange_by_filename',
+        id='arrange_square',
+        text='&Square (by filename)',
+        callback='on_action_arrange_square',
         group='active_when_selection',
     ),
     Action(

beeref/actions/menu_structure.py

@@ -102,7 +102,7 @@
             'arrange_optimal',
             'arrange_horizontal',
             'arrange_vertical',
-            'arrange_by_filename',
+            'arrange_square',
         ],
     },
     {

beeref/scene.py

@@ -183,7 +183,7 @@ def arrange(self, vertical=False):
 
         self.cancel_active_modes()
 
-        items = self.selectedItems(user_only=True)
+        items = sort_by_filename(self.selectedItems(user_only=True))
         if len(items) < 2:
             return
 
@@ -258,15 +258,7 @@ def arrange_optimal(self):
 
         self.undo_stack.push(commands.ArrangeItems(self, items, positions))
 
-    def arrange_by_filename(self):
-        """Order items by filename.
-
-        Items with a filename (ordered by filename) first, then items
-        without a filename but with a save_id follow (ordered by
-        save_id), then remaining items in the order that they have
-        been inserted into the scene.
-        """
-
+    def arrange_square(self):
         self.cancel_active_modes()
         max_width = 0
         max_height = 0

beeref/view.py

@@ -324,8 +324,8 @@ def on_action_arrange_vertical(self):
     def on_action_arrange_optimal(self):
         self.scene.arrange_optimal()
 
-    def on_action_arrange_by_filename(self):
-        self.scene.arrange_by_filename()
+    def on_action_arrange_square(self):
+        self.scene.arrange_square()
 
     def on_action_change_opacity(self):
         images = list(filter(

tests/items/test_items.py

@@ -1,6 +1,6 @@
 from PyQt6 import QtGui
 
-from beeref.items import sort_by_filename, BeePixmapItem
+from beeref.items import sort_by_filename, BeePixmapItem, BeeTextItem
 
 
 def test_sort_by_filename(view):
@@ -38,3 +38,9 @@ def test_sort_by_filename_when_only_by_save_id(view):
     item2 = BeePixmapItem(QtGui.QImage())
     item2.save_id = 33
     assert sort_by_filename([item1, item2]) == [item2, item1]
+
+
+def test_sort_by_filename_deals_with_text_items(view):
+    item1 = BeeTextItem('Foo')
+    item2 = BeeTextItem('Bar')
+    assert len(sort_by_filename([item1, item2])) == 2

tests/test_scene.py

@@ -251,20 +251,21 @@ def test_normalize_size_when_no_items(view):
 
 def test_arrange_horizontal(view):
     item1 = BeePixmapItem(QtGui.QImage())
+    item1.filename = 'foo.png'
     view.scene.addItem(item1)
     item1.setSelected(True)
     item1.setPos(10, -100)
+    item1.crop = QtCore.QRectF(0, 0, 100, 80)
+
     item2 = BeePixmapItem(QtGui.QImage())
+    item2.filename = 'bar.png'
     view.scene.addItem(item2)
     item2.setSelected(True)
     item2.setPos(-10, 40)
-    view.scene.cancel_crop_mode = MagicMock()
+    item2.crop = QtCore.QRectF(0, 0, 100, 80)
 
-    with patch.object(item1, 'bounding_rect_unselected',
-                      return_value=QtCore.QRectF(0, 0, 100, 80)):
-        with patch.object(item2, 'bounding_rect_unselected',
-                          return_value=QtCore.QRectF(0, 0, 100, 80)):
-            view.scene.arrange()
+    view.scene.cancel_crop_mode = MagicMock()
+    view.scene.arrange()
 
     assert item2.pos() == QtCore.QPointF(-50, -30)
     assert item1.pos() == QtCore.QPointF(50, -30)
@@ -273,21 +274,23 @@ def test_arrange_horizontal(view):
 
 def test_arrange_horizontal_with_gap(view, settings):
     settings.setValue('Items/arrange_gap', 6)
+
     item1 = BeePixmapItem(QtGui.QImage())
+    item1.filename = 'foo.png'
     view.scene.addItem(item1)
     item1.setSelected(True)
     item1.setPos(10, -100)
+    item1.crop = QtCore.QRectF(0, 0, 100, 80)
+
     item2 = BeePixmapItem(QtGui.QImage())
+    item2.filename = 'bar.png'
     view.scene.addItem(item2)
     item2.setSelected(True)
     item2.setPos(-10, 40)
-    view.scene.cancel_crop_mode = MagicMock()
+    item2.crop = QtCore.QRectF(0, 0, 100, 80)
 
-    with patch.object(item1, 'bounding_rect_unselected',
-                      return_value=QtCore.QRectF(0, 0, 100, 80)):
-        with patch.object(item2, 'bounding_rect_unselected',
-                          return_value=QtCore.QRectF(0, 0, 100, 80)):
-            view.scene.arrange()
+    view.scene.cancel_crop_mode = MagicMock()
+    view.scene.arrange()
 
     assert item2.pos() == QtCore.QPointF(-50, -30)
     assert item1.pos() == QtCore.QPointF(56, -30)
@@ -296,47 +299,50 @@ def test_arrange_horizontal_with_gap(view, settings):
 
 def test_arrange_vertical(view):
     item1 = BeePixmapItem(QtGui.QImage())
+    item1.filename = 'foo.png'
     view.scene.addItem(item1)
     item1.setSelected(True)
     item1.setPos(10, -100)
+    item1.crop = QtCore.QRectF(0, 0, 100, 80)
+
     item2 = BeePixmapItem(QtGui.QImage())
+    item2.filename = 'bar.png'
     view.scene.addItem(item2)
     item2.setSelected(True)
     item2.setPos(-10, 40)
-    view.scene.cancel_crop_mode = MagicMock()
+    item2.crop = QtCore.QRectF(0, 0, 100, 80)
 
-    with patch.object(item1, 'bounding_rect_unselected',
-                      return_value=QtCore.QRectF(0, 0, 100, 80)):
-        with patch.object(item2, 'bounding_rect_unselected',
-                          return_value=QtCore.QRectF(0, 0, 100, 80)):
-            view.scene.arrange(vertical=True)
+    view.scene.cancel_crop_mode = MagicMock()
+    view.scene.arrange(vertical=True)
 
     assert item1.pos() == QtCore.QPointF(0, -70)
     assert item2.pos() == QtCore.QPointF(0, 10)
-    view.scene.cancel_crop_mode = MagicMock()
+    view.scene.cancel_crop_mode.assert_called_once_with()
 
 
 def test_arrange_vertical_with_gap(view, settings):
     settings.setValue('Items/arrange_gap', 6)
+
     item1 = BeePixmapItem(QtGui.QImage())
+    item1.filename = 'foo.png'
     view.scene.addItem(item1)
     item1.setSelected(True)
     item1.setPos(10, -100)
+    item1.crop = QtCore.QRectF(0, 0, 100, 80)
+
     item2 = BeePixmapItem(QtGui.QImage())
+    item2.filename = 'bar.png'
     view.scene.addItem(item2)
     item2.setSelected(True)
     item2.setPos(-10, 40)
-    view.scene.cancel_crop_mode = MagicMock()
+    item2.crop = QtCore.QRectF(0, 0, 100, 80)
 
-    with patch.object(item1, 'bounding_rect_unselected',
-                      return_value=QtCore.QRectF(0, 0, 100, 80)):
-        with patch.object(item2, 'bounding_rect_unselected',
-                          return_value=QtCore.QRectF(0, 0, 100, 80)):
-            view.scene.arrange(vertical=True)
+    view.scene.cancel_crop_mode = MagicMock()
+    view.scene.arrange(vertical=True)
 
     assert item1.pos() == QtCore.QPointF(0, -70)
     assert item2.pos() == QtCore.QPointF(0, 16)
-    view.scene.cancel_crop_mode = MagicMock()
+    view.scene.cancel_crop_mode.assert_called_once_with()
 
 
 def test_arrange_when_rotated(view):
@@ -429,7 +435,7 @@ def test_arrange_optimal_when_no_items(view):
     view.scene.cancel_crop_mode.assert_called_once_with()
 
 
-def test_arrange_by_filename(view):
+def test_arrange_square(view):
     item1 = BeePixmapItem(QtGui.QImage())
     view.scene.addItem(item1)
     item1.setSelected(True)
@@ -456,7 +462,7 @@ def test_arrange_by_filename(view):
     item4.crop = QtCore.QRectF(0, 0, 100, 80)
 
     view.scene.cancel_crop_mode = MagicMock()
-    view.scene.arrange_by_filename()
+    view.scene.arrange_square()
 
     assert item4.pos() == QtCore.QPointF(-50, -40)
     assert item2.pos() == QtCore.QPointF(60, -30)
@@ -465,7 +471,7 @@ def test_arrange_by_filename(view):
     view.scene.cancel_crop_mode.assert_called_once_with()
 
 
-def test_arrange_by_filename_with_gap(view, settings):
+def test_arrange_square_with_gap(view, settings):
     settings.setValue('Items/arrange_gap', 6)
     item1 = BeePixmapItem(QtGui.QImage())
     view.scene.addItem(item1)
@@ -493,7 +499,7 @@ def test_arrange_by_filename_with_gap(view, settings):
     item4.crop = QtCore.QRectF(0, 0, 100, 80)
 
     view.scene.cancel_crop_mode = MagicMock()
-    view.scene.arrange_by_filename()
+    view.scene.arrange_square()
 
     assert item4.pos() == QtCore.QPointF(-53, -43)
     assert item2.pos() == QtCore.QPointF(63, -33)
@@ -502,9 +508,9 @@ def test_arrange_by_filename_with_gap(view, settings):
     view.scene.cancel_crop_mode.assert_called_once_with()
 
 
-def test_arrange_by_filename_when_no_items(view):
+def test_arrange_square_when_no_items(view):
     view.scene.cancel_crop_mode = MagicMock()
-    view.scene.arrange_by_filename()
+    view.scene.arrange_square()
     view.scene.cancel_crop_mode.assert_called_once_with()
 
 

tests/test_view.py

@@ -1011,9 +1011,9 @@ def test_on_action_arrange_optimal(arrange_mock, view):
     arrange_mock.assert_called_once_with()
 
 
-@patch('beeref.scene.BeeGraphicsScene.arrange_by_filename')
-def test_on_action_arrange_by_filename(arrange_mock, view):
-    view.on_action_arrange_by_filename()
+@patch('beeref.scene.BeeGraphicsScene.arrange_square')
+def test_on_action_arrange_square(arrange_mock, view):
+    view.on_action_arrange_square()
     arrange_mock.assert_called_once_with()