Skip to content

Commit

Permalink
Ensure folder items in sidebar always expandable (#1252)
Browse files Browse the repository at this point in the history 
* FolderManager.File: Only count valid files as children

* Rework add remove and dummy sidebar items

* Revert deleterious change

* Fix CI failure

* Ensure children loaded after adding to expanded empty folder

* Always override add remove  in FolderItem

---------

Co-authored-by: Danielle Foré <[email protected]>
Co-authored-by: Ryan Kornheisl <[email protected]>
  • Loading branch information
@danirabbit @zeebok
3 people authored May 12, 2023
1 parent c60b155 commit 64b5a90
Show file tree
Hide file tree
Showing 3 changed files with 111 additions and 118 deletions.
9 changes: 8 additions & 1 deletion src/FolderManager/File.vala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,10 @@ namespace Scratch.FolderManager {
// checks if we're dealing with a textfile
public bool is_valid_textfile {
get {
if (path.has_prefix (".goutputstream")) {
return false;
}

if (info.get_is_backup ()) {
return false;
}
Expand Down Expand Up @@ -146,7 +150,10 @@ namespace Scratch.FolderManager {
var file_info = new FileInfo ();
while ((file_info = enumerator.next_file ()) != null) {
var child = file.get_child (file_info.get_name ());
_children.add (new File (child.get_path ()));
var child_file = new File (child.get_path ());
if (child_file.is_valid_directory () || child_file.is_valid_textfile) {
_children.add (child_file);
}
}

children_valid = true;
Expand Down
218 changes: 101 additions & 117 deletions src/FolderManager/FolderItem.vala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ namespace Scratch.FolderManager {
public class FolderItem : Item {
private GLib.FileMonitor monitor;
private bool children_loaded = false;
private bool has_dummy;
private Granite.Widgets.SourceList.Item dummy; /* Blank item for expanded empty folders */

public FolderItem (File file, FileView view) requires (file.is_valid_directory) {
Expand All @@ -40,22 +41,11 @@ namespace Scratch.FolderManager {
selectable = false;

dummy = new Granite.Widgets.SourceList.Item ("");
add (dummy);

toggled.connect (() => {
var root = get_root_folder ();
if (!children_loaded && expanded && n_children <= 1 && file.children.size > 0) {
clear ();
add_children ();
if (root != null) {
root.child_folder_loaded (this);
}
// Must add dummy on unexpanded folders else expander will not show
((Granite.Widgets.SourceList.ExpandableItem)this).add (dummy);
has_dummy = true;

children_loaded = true;
} else if (!expanded && root != null) {
root.update_item_status (this); //When toggled closed, update status to reflect hidden contents
}
});
toggled.connect (on_toggled);

try {
monitor = file.file.monitor_directory (GLib.FileMonitorFlags.NONE);
Expand All @@ -65,6 +55,38 @@ namespace Scratch.FolderManager {
}
}

private void on_toggled () {
var root = get_root_folder ();
if (!children_loaded &&
expanded &&
n_children <= 1 &&
file.children.size > 0) {

foreach (var child in file.children) {
Granite.Widgets.SourceList.Item item = null;
if (child.is_valid_directory ()) {
item = new FolderItem (child, view);
} else if (child.is_valid_textfile) {
item = new FileItem (child, view);
}

if (item != null) {
add (item);
}
}

children_loaded = true;
if (root != null) {
root.child_folder_loaded (this);
}
} else if (!expanded &&
root != null &&
root.monitored_repo != null) {
//When toggled closed, update status to reflect hidden contents
root.update_item_status (this);
}
}

public override Gtk.Menu? get_context_menu () {
var contractor_menu = new Gtk.Menu ();

Expand Down Expand Up @@ -186,79 +208,70 @@ namespace Scratch.FolderManager {
return new_item;
}

private void add_children () {
foreach (var child in file.children) {
Granite.Widgets.SourceList.Item item = null;
if (child.is_valid_directory ()) {
item = new FolderItem (child, view);
} else if (child.is_valid_textfile) {
item = new FileItem (child, view);
}

if (item != null) {
add (item);
}
}
}

private void remove_all_children () {
public void remove_all_badges () {
foreach (var child in children) {
remove (child);
remove_badge (child);
}
}

private new void remove (Granite.Widgets.SourceList.Item item) {
private void remove_badge (Granite.Widgets.SourceList.Item item) {
if (item is FolderItem) {
((FolderItem) item).remove_all_children ();
((FolderItem) item).remove_all_badges ();
}

base.remove (item);
item.badge = "";
}

public void remove_all_badges () {
foreach (var child in children) {
remove_badge (child);
public new void add (Granite.Widgets.SourceList.Item item) {
if (has_dummy && n_children == 1) {
((Granite.Widgets.SourceList.ExpandableItem)this).remove (dummy);
has_dummy = false;
}

((Granite.Widgets.SourceList.ExpandableItem)this).add (item);
}

private void remove_badge (Granite.Widgets.SourceList.Item item) {
public new void remove (Granite.Widgets.SourceList.Item item) {
if (item is FolderItem) {
((FolderItem) item).remove_all_badges ();
var folder = (FolderItem)item;
foreach (var child in folder.children) {
folder.remove (child);
}
}

item.badge = "";
view.ignore_next_select = true;
((Granite.Widgets.SourceList.ExpandableItem)this).remove (item);
// Add back dummy if empty unless we are removing a rename item
if (!(item is RenameItem || has_dummy || n_children > 0)) {
((Granite.Widgets.SourceList.ExpandableItem)this).add (dummy);
has_dummy = true;
}
}

public new void clear () {
((Granite.Widgets.SourceList.ExpandableItem)this).clear ();
has_dummy = false;
}

private void on_changed (GLib.File source, GLib.File? dest, GLib.FileMonitorEvent event) {
if (!children_loaded) {
if (source.get_basename ().has_prefix (".goutputstream")) {
return; // Ignore changes due to temp files and streams
}

if (!children_loaded) { // No child items except dummy, child never expanded
/* Empty folder with dummy item will come here even if expanded */
switch (event) {
case GLib.FileMonitorEvent.DELETED:
// This is a pretty intensive operation. For each file deleted, the cache will be
// invalidated and recreated again, from disk. If it turns out users are seeing
// slugishness or slowness when deleting a lot of files, then it might be worth
// storing file.children.size in a variable and subtracting from it with every
// delete
file.invalidate_cache ();

if (file.children.size == 0) {
clear ();
file.invalidate_cache (); //TODO Throttle if required
if (expanded) {
toggled ();
}

break;
case GLib.FileMonitorEvent.CREATED:
if (source.query_exists () == false) {
return;
}

/* Fix adding new file to expanded empty folder */
if (expanded && file.children.size == 0) {
file.invalidate_cache ();
clear ();
add_children ();
children_loaded = true;
file.invalidate_cache (); //TODO Throttle if required
if (expanded) {
toggled ();
}

break;
case FileMonitorEvent.RENAMED:
case FileMonitorEvent.PRE_UNMOUNT:
Expand All @@ -272,28 +285,15 @@ namespace Scratch.FolderManager {

break;
}
} else {
} else { // Child has been expanded ( but could be closed now) and items loaded (or dummy)
// No cache invalidation is needed here because the entire state is kept in the tree
switch (event) {
case GLib.FileMonitorEvent.DELETED:
var children_tmp = new Gee.ArrayList<Granite.Widgets.SourceList.Item> ();
children_tmp.add_all (children);
foreach (var item in children_tmp) {
if (((Item) item).path == source.get_path ()) {
// This is a workaround for SourceList silliness: you cannot remove an item
// without it automatically selecting another one.

view.ignore_next_select = true;
remove (item);
if (file.children.size == 0) {
clear ();
add (dummy);
expanded = false;
children_loaded = false;
}

view.selected = null;
}
// Find item corresponding to deleted file
// Note may not be found if deleted file is not valid for display
var path_item = find_item_for_path (source.get_path ());
if (path_item != null) {
remove (path_item);
}

break;
Expand All @@ -302,32 +302,16 @@ namespace Scratch.FolderManager {
return;
}

// Temporary files from GLib that are present when saving a file
if (source.get_basename ().has_prefix (".goutputstream")) {
return;
}

var file = new File (source.get_path ());
var exists = false;
foreach (var item in children) {
if (((Item) item).path == file.path) {
exists = true;
break;
}
}

Item? item = null;

if (!exists) {
var path_item = find_item_for_path (source.get_path ());
if (path_item == null) {
var file = new File (source.get_path ());
if (file.is_valid_directory ()) {
item = new FolderItem (file, view);
path_item = new FolderItem (file, view);
} else if (!file.is_temporary) {
item = new FileItem (file, view);
path_item = new FileItem (file, view);
}
}

if (item != null) {
add (item);
add (path_item);
}

break;
Expand All @@ -340,7 +324,6 @@ namespace Scratch.FolderManager {
case FileMonitorEvent.MOVED_IN:
case FileMonitorEvent.MOVED_OUT:
case FileMonitorEvent.ATTRIBUTE_CHANGED:

break;
}
}
Expand All @@ -356,6 +339,17 @@ namespace Scratch.FolderManager {
}
}

private FolderManager.Item? find_item_for_path (string path) {
foreach (var item in children) {
// Item could be dummy
if ((item is FolderManager.Item) && ((FolderManager.Item) item).path == path) {
return (FolderManager.Item)item;
}
}

return null;
}

private void on_add_new (bool is_folder) {
if (!file.is_executable) {
// This is necessary to avoid infinite loop below
Expand All @@ -371,27 +365,19 @@ namespace Scratch.FolderManager {
new_file = file.file.get_child (("%s %d").printf (name, n));
n++;
}

expanded = true;
var rename_item = new RenameItem (new_file.get_basename (), is_folder);
if (file.children.size == 0) {
clear (); /* Remove dummy item */
}

add (rename_item);

/* Start editing after finishing signal handler */
GLib.Idle.add (() => {
view.start_editing_item (rename_item);

/* Need to poll view editing as no signal is generated when canceled (Granite bug) */
Timeout.add (200, () => {
if (view.editing) {
return Source.CONTINUE;
} else {
var new_name = rename_item.name;
view.ignore_next_select = true;
remove (rename_item);
try {
var gfile = file.file.get_child_for_display_name (new_name);
if (is_folder) {
Expand All @@ -402,10 +388,8 @@ namespace Scratch.FolderManager {
}
} catch (Error e) {
warning (e.message);
/* Replace dummy if file creation fails */
if (file.children.size == 0) {
add (dummy);
}
} finally {
remove (rename_item);
}
}

Expand Down
2 changes: 2 additions & 0 deletions src/FolderManager/Item.vala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ namespace Scratch.FolderManager {
return 1;
}

assert (a is Item && b is Item); //Ensure more informative error message

return File.compare (((Item)a).file, ((Item)b).file);
}

Expand Down

0 comments on commit 64b5a90

Please sign in to comment.