shifathosn
/
photocraft
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(engine): select neighbouring layer after deletion (#2189) 

Co-authored-by: kotulsky <7805437+kotulsky@users.noreply.github.com>
Co-authored-by: Brandon Thomas <echelon@gmail.com>
This commit is contained in:
kotulsky 2026-10-10 08:01:55 +02:00 committed by GitHub
parent 8a80430c40
commit 8b1bb0fd05
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 100 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
crates/engine/src/commands.rs
View File

@ -568,12 +568,13 @@ fn build() -> Vec<CommandSpec> {
}
let id = layer_param(s, p)?;
s.edit("Delete Layer", |doc, active| {
let neighbours = crate::layer_multi_cmds::deletion_neighbours(doc, id);
doc.remove(id).ok_or(EngineError::NoLayer(id))?;
if doc.layers.is_empty() {
return Err(EngineError::Other("a document must keep at least one layer".into()));
}
if *active == Some(id) {
*active = doc.top_layer();
if active.is_some_and(|current| doc.layer(current).is_none()) {
*active = neighbours.into_iter().find(|candidate| doc.layer(*candidate).is_some());
}
Ok(())
})?;

34
crates/engine/src/layer_multi_cmds.rs
View File

@ -1121,23 +1121,45 @@ fn merge_layers(s: &mut Session) -> Result<Value> {
Ok(json!({ "layer": mid.0 }))
}
/// Next visible layer row below the deleted target, then rows above in reverse order.
/// Group headers precede their children in the UI, unlike Document::walk's storage order.
pub(crate) fn deletion_neighbours(doc: &Document, target: LayerId) -> Vec<LayerId> {
let rows = |expand_all: bool| {
let mut stack: Vec<&Layer> = doc.layers.iter().collect();
let mut order = Vec::new();
while let Some(layer) = stack.pop() {
order.push(layer.id);
if let LayerContent::Group(group) = &layer.content
&& (expand_all || group.expanded)
{
stack.extend(group.children.iter());
}
}
order
};
let mut order = rows(false);
if !order.contains(&target) {
order = rows(true);
}
let Some(index) = order.iter().position(|id| *id == target) else {
return Vec::new();
};
order.iter().skip(index + 1).chain(order.iter().take(index).rev()).copied().collect()
}
/// Delete Layer with several layers selected.
pub fn delete_selected(s: &mut Session) -> Result<Value> {
let sel = selected(s);
s.edit("Delete Layers", |doc, active| {
let ids = top_level(doc, &sel);
let below = ids.first().and_then(|id| {
let order: Vec<LayerId> = doc.walk().into_iter().map(|(_, _, l)| l.id).collect();
let i = order.iter().position(|x| x == id)?;
order[..i].iter().rev().find(|x| !sel.contains(x)).copied()
});
let neighbours = active.map(|id| deletion_neighbours(doc, id)).unwrap_or_default();
for id in &ids {
doc.remove(*id).ok_or(EngineError::NoLayer(*id))?;
}
if doc.layers.is_empty() {
return Err(EngineError::Other("a document must keep at least one layer".into()));
}
*active = below.filter(|b| doc.layer(*b).is_some()).or_else(|| doc.top_layer());
*active = neighbours.into_iter().find(|candidate| doc.layer(*candidate).is_some());
Ok(())
})?;
Ok(json!({"deleted": sel.len()}))

69
crates/engine/src/tests.rs
View File

@ -1284,3 +1284,72 @@ fn document_inspect_and_activate_invalid_index() {
let err_act = s.execute("document.activate", json!({"document": 9})).unwrap_err();
assert_eq!(err_act.to_string(), "no document at index 9");
}
#[test]
fn delete_layer_selects_neighbour_and_undo_restores_layer() {
for deleted_index in 0..3 {
let mut s = session_with_doc();
let ids = ["bottom", "middle", "top"].map(|name| LayerId(s.execute("layer.new.layer", json!({"name":name})).unwrap()["layer"].as_u64().unwrap()));
if deleted_index == 0 {
let background = s.active().unwrap().doc.layers[0].id;
s.execute("layer.delete", json!({"layer":background.0})).unwrap();
}
s.execute("layer.select", json!({"layer":ids[deleted_index].0})).unwrap();
s.execute("layer.delete", json!({})).unwrap();
let neighbour = if deleted_index == 0 { ids[1] } else { ids[deleted_index - 1] };
assert_eq!(s.active().unwrap().active_layer, Some(neighbour));
assert_eq!(s.active().unwrap().selected_layers, vec![neighbour]);
assert!(s.active().unwrap().doc.layer(ids[deleted_index]).is_none());
s.undo();
assert!(s.active().unwrap().doc.layer(ids[deleted_index]).is_some());
assert!(s.active().unwrap().active_layer.is_some_and(|id| s.active().unwrap().doc.layer(id).is_some()));
s.redo();
assert_eq!(s.active().unwrap().active_layer, Some(neighbour));
}
}
#[test]
fn delete_multiple_layers_selects_next_survivor() {
let mut s = session_with_doc();
let ids = ["bottom", "middle", "top"].map(|name| LayerId(s.execute("layer.new.layer", json!({"name":name})).unwrap()["layer"].as_u64().unwrap()));
s.execute("layer.select", json!({"layer":ids[1].0})).unwrap();
s.execute("layer.select", json!({"layer":ids[2].0,"mode":"add"})).unwrap();
s.execute("layer.delete", json!({})).unwrap();
assert_eq!(s.active().unwrap().active_layer, Some(ids[0]));
assert!(s.active().unwrap().doc.layer(ids[1]).is_none());
assert!(s.active().unwrap().doc.layer(ids[2]).is_none());
s.undo();
assert_eq!(s.active().unwrap().active_layer, Some(ids[2]));
}
#[test]
fn delete_layer_neighbours_follow_group_row_order() {
let mut s = session_with_doc();
let background = s.active().unwrap().active_layer.unwrap();
let lower = LayerId(s.execute("layer.new.layer", json!({"name":"Below group"})).unwrap()["layer"].as_u64().unwrap());
let child = LayerId(s.execute("layer.new.layer", json!({"name":"Child"})).unwrap()["layer"].as_u64().unwrap());
let group = LayerId(s.execute("layer.groupLayers", json!({"name":"Group"})).unwrap()["layer"].as_u64().unwrap());
s.execute("layer.select", json!({"layer":child.0})).unwrap();
s.execute("layer.delete", json!({})).unwrap();
assert_eq!(s.active().unwrap().active_layer, Some(lower), "the next row below a group's final child is outside the group");
s.undo();
s.execute("layer.delete", json!({"layer":background.0})).unwrap();
s.execute("layer.setExpanded", json!({"layer":group.0,"expanded":false})).unwrap();
s.execute("layer.select", json!({"layer":lower.0})).unwrap();
s.execute("layer.delete", json!({})).unwrap();
assert_eq!(s.active().unwrap().active_layer, Some(group), "fallback above uses the collapsed group header, not its hidden child");
}
#[test]
fn delete_layer_preserves_an_unaffected_active_layer_and_last_layer_guard() {
let mut s = session_with_doc();
let background = s.active().unwrap().active_layer.unwrap();
let other = LayerId(s.execute("layer.new.layer", json!({"name":"Other"})).unwrap()["layer"].as_u64().unwrap());
s.execute("layer.delete", json!({"layer":background.0})).unwrap();
assert_eq!(s.active().unwrap().active_layer, Some(other));
let before = s.active().unwrap().history.past_len();
assert!(s.execute("layer.delete", json!({})).is_err());
assert_eq!(s.active().unwrap().doc.layer_count(), 1);
assert_eq!(s.active().unwrap().active_layer, Some(other));
assert_eq!(s.active().unwrap().history.past_len(), before);
}