From edd00096b6271ab56e12029fe02cd1c2401d0394 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Hatcher?= Date: Fri, 14 Feb 2025 11:23:34 +0100 Subject: [PATCH] FIX: Minor fixes in column/row styles Most notably deleting the formatting does not change width/height --- base/src/test/user_model/test_column_style.rs | 52 ++++++++ .../test_delete_row_column_formatting.rs | 115 +++++++++++++++++- base/src/user_model/common.rs | 84 +++++++------ base/src/user_model/history.rs | 2 +- base/src/worksheet.rs | 14 ++- 5 files changed, 219 insertions(+), 48 deletions(-) diff --git a/base/src/test/user_model/test_column_style.rs b/base/src/test/user_model/test_column_style.rs index 80a658a..82e9e47 100644 --- a/base/src/test/user_model/test_column_style.rs +++ b/base/src/test/user_model/test_column_style.rs @@ -403,3 +403,55 @@ fn set_row_style_then_cell() { let style = model.get_cell_style(0, 12, 7).unwrap(); assert_eq!(style.fill.bg_color, Some("#333444".to_string())); } + +#[test] +fn column_style_then_row_alignment() { + let mut model = UserModel::new_empty("model", "en", "UTC").unwrap(); + let column_g_range = Area { + sheet: 0, + row: 1, + column: 7, + width: 1, + height: LAST_ROW, + }; + let row_3_range = Area { + sheet: 0, + row: 3, + column: 1, + width: LAST_COLUMN, + height: 1, + }; + model + .update_range_style(&column_g_range, "fill.bg_color", "#555666") + .unwrap(); + model + .update_range_style(&row_3_range, "alignment.horizontal", "center") + .unwrap(); + // check the row alignment does not affect the column style + let style = model.get_cell_style(0, 3, 7).unwrap(); + assert_eq!(style.fill.bg_color, Some("#555666".to_string())); +} + +#[test] +fn column_style_then_width() { + let mut model = UserModel::new_empty("model", "en", "UTC").unwrap(); + let column_g_range = Area { + sheet: 0, + row: 1, + column: 7, + width: 1, + height: LAST_ROW, + }; + model + .update_range_style(&column_g_range, "fill.bg_color", "#555666") + .unwrap(); + model + .set_column_width(0, 7, DEFAULT_COLUMN_WIDTH * 2.0) + .unwrap(); + + // Check column width worked: + assert_eq!( + model.get_column_width(0, 7).unwrap(), + DEFAULT_COLUMN_WIDTH * 2.0 + ); +} diff --git a/base/src/test/user_model/test_delete_row_column_formatting.rs b/base/src/test/user_model/test_delete_row_column_formatting.rs index 7cc8828..fc6248a 100644 --- a/base/src/test/user_model/test_delete_row_column_formatting.rs +++ b/base/src/test/user_model/test_delete_row_column_formatting.rs @@ -1,7 +1,7 @@ #![allow(clippy::unwrap_used)] use crate::{ - constants::{DEFAULT_COLUMN_WIDTH, LAST_COLUMN, LAST_ROW}, + constants::{DEFAULT_COLUMN_WIDTH, DEFAULT_ROW_HEIGHT, LAST_COLUMN, LAST_ROW}, expressions::types::Area, UserModel, }; @@ -121,8 +121,13 @@ fn column_width() { .update_range_style(&column_g_range, "fill.bg_color", "#555666") .unwrap(); + // Delete the column formatting model.range_clear_formatting(&column_g_range).unwrap(); - assert_eq!(model.get_column_width(0, 7).unwrap(), DEFAULT_COLUMN_WIDTH); + // This does not change the column width + assert_eq!( + model.get_column_width(0, 7).unwrap(), + 2.0 * DEFAULT_COLUMN_WIDTH + ); model.undo().unwrap(); assert_eq!( @@ -130,5 +135,109 @@ fn column_width() { 2.0 * DEFAULT_COLUMN_WIDTH ); model.redo().unwrap(); - assert_eq!(model.get_column_width(0, 7).unwrap(), DEFAULT_COLUMN_WIDTH); + assert_eq!( + model.get_column_width(0, 7).unwrap(), + 2.0 * DEFAULT_COLUMN_WIDTH + ); +} + +#[test] +fn column_row_style_undo() { + let mut model = UserModel::new_empty("model", "en", "UTC").unwrap(); + model + .set_column_width(0, 7, DEFAULT_COLUMN_WIDTH * 2.0) + .unwrap(); + + let column_g_range = Area { + sheet: 0, + row: 1, + column: 7, + width: 1, + height: LAST_ROW, + }; + + let row_123_range = Area { + sheet: 0, + row: 123, + column: 1, + width: LAST_COLUMN, + height: 1, + }; + + let delete_range = Area { + sheet: 0, + row: 120, + column: 5, + width: 20, + height: 20, + }; + + // Set the style of the whole column + model + .update_range_style(&column_g_range, "fill.bg_color", "#555666") + .unwrap(); + + model + .update_range_style(&row_123_range, "fill.bg_color", "#111222") + .unwrap(); + + model.range_clear_formatting(&delete_range).unwrap(); + + // check G123 is empty + let style = model.get_cell_style(0, 123, 7).unwrap(); + assert_eq!(style.fill.bg_color, None); + + // uno clear formatting + model.undo().unwrap(); + + // G123 has the row style + let style = model.get_cell_style(0, 123, 7).unwrap(); + assert_eq!(style.fill.bg_color, Some("#111222".to_owned())); + + // undo twice + model.undo().unwrap(); + model.undo().unwrap(); + + // check G123 is empty + let style = model.get_cell_style(0, 123, 7).unwrap(); + assert_eq!(style.fill.bg_color, None); +} + +#[test] +fn column_row_row_height_undo() { + let mut model = UserModel::new_empty("model", "en", "UTC").unwrap(); + + let column_g_range = Area { + sheet: 0, + row: 1, + column: 7, + width: 1, + height: LAST_ROW, + }; + + let row_3_range = Area { + sheet: 0, + row: 3, + column: 1, + width: LAST_COLUMN, + height: 1, + }; + + model + .update_range_style(&column_g_range, "fill.bg_color", "#555666") + .unwrap(); + + model + .set_row_height(0, 3, DEFAULT_ROW_HEIGHT * 2.0) + .unwrap(); + + model + .update_range_style(&row_3_range, "fill.bg_color", "#111222") + .unwrap(); + + model.undo().unwrap(); + + // check G3 has the column style + let style = model.get_cell_style(0, 3, 7).unwrap(); + assert_eq!(style.fill.bg_color, Some("#555666".to_string())); } diff --git a/base/src/user_model/common.rs b/base/src/user_model/common.rs index a4b9942..936ab1a 100644 --- a/base/src/user_model/common.rs +++ b/base/src/user_model/common.rs @@ -6,7 +6,7 @@ use csv::{ReaderBuilder, WriterBuilder}; use serde::{Deserialize, Serialize}; use crate::{ - constants::{self, DEFAULT_COLUMN_WIDTH, DEFAULT_ROW_HEIGHT, LAST_COLUMN, LAST_ROW}, + constants::{self, DEFAULT_ROW_HEIGHT, LAST_COLUMN, LAST_ROW}, expressions::{ types::{Area, CellReferenceIndex}, utils::{is_valid_column_number, is_valid_row}, @@ -653,18 +653,6 @@ impl UserModel { column, old_value: Box::new(old_value), }); - let column_width = self.model.get_column_width(sheet, column)?; - if column_width != DEFAULT_COLUMN_WIDTH { - self.model - .set_column_width(sheet, column, DEFAULT_COLUMN_WIDTH)?; - - diff_list.push(Diff::SetColumnWidth { - sheet, - column, - new_value: DEFAULT_COLUMN_WIDTH, - old_value: column_width, - }); - } let data_rows: Vec = self .model @@ -688,7 +676,7 @@ impl UserModel { sheet, row, column, - old_style: Box::new(old_style), + old_style: Box::new(Some(old_style)), }); } else { let old_style = self.model.get_style_for_cell(sheet, row, column)?; @@ -701,7 +689,7 @@ impl UserModel { sheet, row, column, - old_style: Box::new(old_style), + old_style: Box::new(None), }); } } @@ -718,7 +706,7 @@ impl UserModel { sheet, row: row.r, column, - old_style: Box::new(old_style), + old_style: Box::new(Some(old_style)), }); } else { let old_style = self.model.get_style_for_cell(sheet, row.r, column)?; @@ -731,7 +719,7 @@ impl UserModel { sheet, row: row.r, column, - old_style: Box::new(old_style), + old_style: Box::new(None), }); } } @@ -770,7 +758,7 @@ impl UserModel { sheet, row, column, - old_style: Box::new(old_style), + old_style: Box::new(Some(old_style)), }); } else { let old_style = self.model.get_style_for_cell(sheet, row, column)?; @@ -783,7 +771,7 @@ impl UserModel { sheet, row, column, - old_style: Box::new(old_style), + old_style: Box::new(None), }); } } @@ -825,7 +813,7 @@ impl UserModel { sheet, row, column, - old_style: Box::new(old_style), + old_style: Box::new(Some(old_style)), }); } else { let old_style = self.model.get_style_for_cell(sheet, row, column)?; @@ -838,7 +826,7 @@ impl UserModel { sheet, row, column, - old_style: Box::new(old_style), + old_style: Box::new(None), }); } } @@ -1191,22 +1179,8 @@ impl UserModel { } } else if range.column == 1 && range.width == LAST_COLUMN { // Full rows + let styled_columns = &self.model.workbook.worksheet(sheet)?.cols.clone(); for row in range.row..range.row + range.height { - // First update the style of the row - let old_style = self.model.get_row_style(sheet, row)?; - let style = match old_style.as_ref() { - Some(s) => s, - None => &Style::default(), - }; - let style = update_style(style, style_path, value)?; - self.model.set_row_style(sheet, row, &style)?; - diff_list.push(Diff::SetRowStyle { - sheet, - row, - old_value: Box::new(old_style), - new_value: Box::new(style), - }); - // Now update style in all cells that are not empty let columns: Vec = self .model @@ -1226,8 +1200,35 @@ impl UserModel { &mut diff_list, )?; } - // since rows take precedence over columns we do not need - // to update the styles in all cells that have a column style + + // We need to go through all the cells that have a column style and merge them + for col in styled_columns.iter() { + for column in col.min..col.max + 1 { + self.update_single_cell_style( + sheet, + row, + column, + style_path, + value, + &mut diff_list, + )?; + } + } + + // Finally update the style of the row + let old_style = self.model.get_row_style(sheet, row)?; + let style = match old_style.as_ref() { + Some(s) => s, + None => &Style::default(), + }; + let style = update_style(style, style_path, value)?; + self.model.set_row_style(sheet, row, &style)?; + diff_list.push(Diff::SetRowStyle { + sheet, + row, + old_value: Box::new(old_style), + new_value: Box::new(style), + }); } } else { for row in range.row..range.row + range.height { @@ -2036,8 +2037,11 @@ impl UserModel { column, old_style, } => { - self.model - .set_cell_style(*sheet, *row, *column, old_style)?; + if let Some(value) = old_style.as_ref() { + self.model.set_cell_style(*sheet, *row, *column, value)?; + } else { + self.model.cell_clear_all(*sheet, *row, *column)?; + } } Diff::DeleteSheet { sheet, old_data } => { needs_evaluation = true; diff --git a/base/src/user_model/history.rs b/base/src/user_model/history.rs index f37b62b..53268e1 100644 --- a/base/src/user_model/history.rs +++ b/base/src/user_model/history.rs @@ -43,7 +43,7 @@ pub(crate) enum Diff { sheet: u32, row: i32, column: i32, - old_style: Box