Skip to content

Commit

Permalink
Merge pull request #1517 from quadratichq/fix-delete-between-columns
Browse files Browse the repository at this point in the history 
Delete works properly with column/row selections
  • Loading branch information
@davidkircos
davidkircos committed Jul 8, 2024
2 parents a323ceb + 091694f commit 8fb52fd
Show file tree
Hide file tree
Showing 5 changed files with 219 additions and 32 deletions.
86 changes: 55 additions & 31 deletions quadratic-core/src/controller/operations/cell_value.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,34 +116,13 @@ impl GridController {
pub fn delete_cells_operations(&self, selection: &Selection) -> Vec<Operation> {
let mut ops = vec![];
if let Some(sheet) = self.try_sheet(selection.sheet_id) {
// Find all cells with values.
if let Some(values) = sheet.selection(selection, None, true) {
// Find the minimum x and y values of the selection, which will
// populate CellValues.
let mut min_x = i64::MAX;
let mut min_y = i64::MAX;
let mut cell_values = CellValues::new(0, 0);
values.iter().for_each(|(pos, _value)| {
min_x = min_x.min(pos.x);
min_y = min_y.min(pos.y);
let rects = sheet.selection_rects_values(selection);
for rect in rects {
let cell_values = CellValues::new(rect.width(), rect.height());
ops.push(Operation::SetCellValues {
sheet_pos: (rect.min.x, rect.min.y, selection.sheet_id).into(),
values: cell_values,
});
if min_x != i64::MAX && min_y != i64::MAX && !values.is_empty() {
values.iter().for_each(|(pos, _value)| {
cell_values.set(
(pos.x - min_x) as u32,
(pos.y - min_y) as u32,
CellValue::Blank,
);
});
ops.push(Operation::SetCellValues {
sheet_pos: SheetPos {
x: min_x,
y: min_y,
sheet_id: selection.sheet_id,
},
values: cell_values,
});
}
}
};
ops
Expand Down Expand Up @@ -319,11 +298,56 @@ mod test {
y: 2,
sheet_id
},
values: CellValues::from_cell_value_vec(vec![vec![
CellValue::Blank,
CellValue::Blank
]])
values: CellValues::new(2, 1)
}]
);
}

#[test]
fn delete_columns() {
let mut gc = GridController::test();
let sheet_id = gc.sheet_ids()[0];
let sheet_pos = SheetPos {
x: 1,
y: 2,
sheet_id,
};
gc.set_cell_value(sheet_pos, "hello".to_string(), None);

let sheet_pos_2 = SheetPos {
x: 2,
y: 2,
sheet_id,
};
gc.set_code_cell(
sheet_pos_2,
CodeCellLanguage::Formula,
"5 + 5".to_string(),
None,
);
let selection = Selection::columns(&[1, 2], sheet_id);
let operations = gc.delete_cells_operations(&selection);
assert_eq!(operations.len(), 2);
assert_eq!(
operations,
vec![
Operation::SetCellValues {
sheet_pos: SheetPos {
x: 1,
y: 2,
sheet_id
},
values: CellValues::new(1, 1)
},
Operation::SetCellValues {
sheet_pos: SheetPos {
x: 2,
y: 2,
sheet_id
},
values: CellValues::new(1, 1)
}
]
);
}
}
3 changes: 2 additions & 1 deletion quadratic-core/src/grid/sheet/rendering.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,8 @@ impl Sheet {
let is_percentage = numeric_format.as_ref().is_some_and(|numeric_format| {
numeric_format.kind == NumericFormatKind::Percentage
});
let numeric_decimals = self.calculate_decimal_places(Pos { x, y }, is_percentage);
let numeric_decimals =
self.calculate_decimal_places(Pos { x, y }, is_percentage);
let numeric_commas = column.numeric_commas.get(y).or(format.numeric_commas);

// if align is not set, set it to right only for numbers
Expand Down
98 changes: 98 additions & 0 deletions quadratic-core/src/grid/sheet/selection.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,43 @@ impl Sheet {
}
cells.into_iter().collect()
}

/// Returns a vec of Rects for a selection. This is useful for creating
/// Operation::SetCellValues so we don't overlap areas that are not
/// selected.
///
/// todo: this returns CodeRuns bounds as well. We probably should make the
/// CodeRuns optional as they're not needed for things like
/// delete_cell_operations operations. But it doesn't do any harm.
pub fn selection_rects_values(&self, selection: &Selection) -> Vec<Rect> {
let mut rects = vec![];
if selection.all {
if let GridBounds::NonEmpty(bounds) = self.bounds(false) {
rects.push(bounds);
}
}

if let Some(columns) = selection.columns.as_ref() {
for x in columns.iter() {
if let Some((min, max)) = self.column_bounds(*x, false) {
rects.push(Rect::new(*x, min, *x, max));
}
}
}

if let Some(rows) = selection.rows.as_ref() {
for y in rows.iter() {
if let Some((min, max)) = self.row_bounds(*y, false) {
rects.push(Rect::new(min, *y, max, *y));
}
}
}

if let Some(rects_selection) = selection.rects.as_ref() {
rects.extend(rects_selection.iter().cloned());
}
rects
}
}

#[cfg(test)]
Expand Down Expand Up @@ -777,4 +814,65 @@ mod tests {
.iter()
.any(|(pos, format)| *pos == Pos { x: 1, y: 1 } && format.bold == Some(false)));
}

#[test]
fn test_selection_rects_values() {
let mut sheet = Sheet::test();
let sheet_id = sheet.id;

let selection = Selection {
sheet_id,
..Default::default()
};
assert_eq!(sheet.selection_rects_values(&selection), vec![]);

sheet.test_set_values(0, 0, 2, 2, vec!["1", "2", "a", "b"]);
sheet.test_set_code_run_array(-1, -1, vec!["c", "d", "e"], true);

let selection = Selection {
sheet_id,
rects: Some(vec![Rect::from_numbers(-4, -4, 10, 10)]),
..Default::default()
};
assert_eq!(
sheet.selection_rects_values(&selection),
vec![Rect::from_numbers(-4, -4, 10, 10)]
);

let selection = Selection {
sheet_id,
columns: Some(vec![-1, 0]),
..Default::default()
};
// note, this includes the CodeRuns bounds as well because of a
// limitation in sheet.column_bounds
assert_eq!(
sheet.selection_rects_values(&selection),
vec![Rect::new(-1, -1, -1, 1), Rect::new(0, 0, 0, 1)]
);

let selection = Selection {
sheet_id,
columns: Some(vec![5, 6]),
..Default::default()
};
assert_eq!(sheet.selection_rects_values(&selection), vec![]);

let selection = Selection {
sheet_id,
rows: Some(vec![-1, 0]),
..Default::default()
};
assert_eq!(
sheet.selection_rects_values(&selection),
vec![Rect::new(-1, -1, -1, -1), Rect::new(-1, 0, 1, 0)]
);

let selection = Selection {
sheet_id,
rows: Some(vec![-10, -11]),
..Default::default()
};
assert_eq!(sheet.selection_rects_values(&selection), vec![]);
}
}
2 changes: 2 additions & 0 deletions quadratic-core/src/position.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ pub struct Rect {
pub max: Pos,
}
impl Rect {
/// Creates a rect from two x, y positions
pub fn new(x0: i64, y0: i64, x1: i64, y1: i64) -> Rect {
Rect {
min: Pos { x: x0, y: y0 },
Expand Down Expand Up @@ -136,6 +137,7 @@ impl Rect {
}
}

/// Creates a rectangle from one x, y position and a width and height
pub fn from_numbers(x: i64, y: i64, w: i64, h: i64) -> Rect {
Rect {
min: Pos { x, y },
Expand Down
62 changes: 62 additions & 0 deletions quadratic-core/src/selection.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,32 @@ impl Selection {
}
}

/// Creates a selection with columns
pub fn columns(columns: &[i64], sheet_id: SheetId) -> Self {
Selection {
sheet_id,
x: columns[0],
y: 0,
rects: None,
rows: None,
columns: Some(columns.to_vec()),
all: false,
}
}

/// Creates a selection with rows
pub fn rows(rows: &[i64], sheet_id: SheetId) -> Self {
Selection {
sheet_id,
x: 0,
y: rows[0],
rects: None,
rows: Some(rows.to_vec()),
columns: None,
all: false,
}
}

/// Creates a selection via single rect
pub fn rect(rect: Rect, sheet_id: SheetId) -> Self {
Selection {
Expand Down Expand Up @@ -482,4 +508,40 @@ mod test {
// all is always count = 1
assert_eq!(selection.count(), 1);
}

#[test]
fn selection_columns() {
let sheet_id = SheetId::test();
let selection = Selection::columns(&[1, 2, 3], sheet_id);
assert_eq!(
selection,
Selection {
sheet_id,
x: 1,
y: 0,
rects: None,
rows: None,
columns: Some(vec![1, 2, 3]),
all: false
}
);
}

#[test]
fn selection_rows() {
let sheet_id = SheetId::test();
let selection = Selection::rows(&[1, 2, 3], sheet_id);
assert_eq!(
selection,
Selection {
sheet_id,
x: 0,
y: 1,
rects: None,
rows: Some(vec![1, 2, 3]),
columns: None,
all: false
}
);
}
}

0 comments on commit 8fb52fd

Please sign in to comment.