FIX: [App]: Borders done right

base/src/test/user_model/test_border.rs

@@ -51,6 +51,84 @@ fn borders_all() {
         }
     }
 
+    // let's check the borders around
+    {
+        let row = 4;
+        for column in 6..9 {
+            let style = model.get_cell_style(0, row, column).unwrap();
+            let border_item = BorderItem {
+                style: BorderStyle::Thin,
+                color: Some("#FF5566".to_string()),
+            };
+            let expected_border = Border {
+                diagonal_up: false,
+                diagonal_down: false,
+                left: None,
+                right: None,
+                top: None,
+                bottom: Some(border_item.clone()),
+                diagonal: None,
+            };
+            assert_eq!(style.border, expected_border);
+        }
+        let row = 9;
+        for column in 6..9 {
+            let style = model.get_cell_style(0, row, column).unwrap();
+            let border_item = BorderItem {
+                style: BorderStyle::Thin,
+                color: Some("#FF5566".to_string()),
+            };
+            let expected_border = Border {
+                diagonal_up: false,
+                diagonal_down: false,
+                left: None,
+                right: None,
+                top: Some(border_item.clone()),
+                bottom: None,
+                diagonal: None,
+            };
+            assert_eq!(style.border, expected_border);
+        }
+    }
+    {
+        let column = 5;
+        for row in 5..9 {
+            let style = model.get_cell_style(0, row, column).unwrap();
+            let border_item = BorderItem {
+                style: BorderStyle::Thin,
+                color: Some("#FF5566".to_string()),
+            };
+            let expected_border = Border {
+                diagonal_up: false,
+                diagonal_down: false,
+                left: None,
+                right: Some(border_item.clone()),
+                top: None,
+                bottom: None,
+                diagonal: None,
+            };
+            assert_eq!(style.border, expected_border);
+        }
+        let column = 9;
+        for row in 5..9 {
+            let style = model.get_cell_style(0, row, column).unwrap();
+            let border_item = BorderItem {
+                style: BorderStyle::Thin,
+                color: Some("#FF5566".to_string()),
+            };
+            let expected_border = Border {
+                diagonal_up: false,
+                diagonal_down: false,
+                left: Some(border_item.clone()),
+                right: None,
+                top: None,
+                bottom: None,
+                diagonal: None,
+            };
+            assert_eq!(style.border, expected_border);
+        }
+    }
+
     // Lets remove all of them:
     let border_area: BorderArea = serde_json::from_str(
         r##"{
@@ -63,8 +141,8 @@ fn borders_all() {
     )
     .unwrap();
     model.set_area_with_border(range, &border_area).unwrap();
-    for row in 5..9 {
+    for row in 4..10 {
-        for column in 6..9 {
+        for column in 5..10 {
             let style = model.get_cell_style(0, row, column).unwrap();
             let expected_border = Border {
                 diagonal_up: false,
@@ -229,6 +307,84 @@ fn borders_outer() {
         };
         assert_eq!(style.border, expected_border);
     }
+
+    // let's check the borders around
+    {
+        let row = 4;
+        for column in 6..9 {
+            let style = model.get_cell_style(0, row, column).unwrap();
+            let border_item = BorderItem {
+                style: BorderStyle::Thin,
+                color: Some("#FF5566".to_string()),
+            };
+            let expected_border = Border {
+                diagonal_up: false,
+                diagonal_down: false,
+                left: None,
+                right: None,
+                top: None,
+                bottom: Some(border_item.clone()),
+                diagonal: None,
+            };
+            assert_eq!(style.border, expected_border);
+        }
+        let row = 9;
+        for column in 6..9 {
+            let style = model.get_cell_style(0, row, column).unwrap();
+            let border_item = BorderItem {
+                style: BorderStyle::Thin,
+                color: Some("#FF5566".to_string()),
+            };
+            let expected_border = Border {
+                diagonal_up: false,
+                diagonal_down: false,
+                left: None,
+                right: None,
+                top: Some(border_item.clone()),
+                bottom: None,
+                diagonal: None,
+            };
+            assert_eq!(style.border, expected_border);
+        }
+    }
+    {
+        let column = 5;
+        for row in 5..9 {
+            let style = model.get_cell_style(0, row, column).unwrap();
+            let border_item = BorderItem {
+                style: BorderStyle::Thin,
+                color: Some("#FF5566".to_string()),
+            };
+            let expected_border = Border {
+                diagonal_up: false,
+                diagonal_down: false,
+                left: None,
+                right: Some(border_item.clone()),
+                top: None,
+                bottom: None,
+                diagonal: None,
+            };
+            assert_eq!(style.border, expected_border);
+        }
+        let column = 9;
+        for row in 5..9 {
+            let style = model.get_cell_style(0, row, column).unwrap();
+            let border_item = BorderItem {
+                style: BorderStyle::Thin,
+                color: Some("#FF5566".to_string()),
+            };
+            let expected_border = Border {
+                diagonal_up: false,
+                diagonal_down: false,
+                left: Some(border_item.clone()),
+                right: None,
+                top: None,
+                bottom: None,
+                diagonal: None,
+            };
+            assert_eq!(style.border, expected_border);
+        }
+    }
 }
 
 #[test]
@@ -275,6 +431,72 @@ fn borders_top() {
             assert_eq!(style.border, expected_border);
         }
     }
+
+    // let's check the borders around
+    {
+        let row = 4;
+        for column in 6..9 {
+            let style = model.get_cell_style(0, row, column).unwrap();
+            let border_item = BorderItem {
+                style: BorderStyle::Thin,
+                color: Some("#FF5566".to_string()),
+            };
+            let expected_border = Border {
+                diagonal_up: false,
+                diagonal_down: false,
+                left: None,
+                right: None,
+                top: None,
+                bottom: Some(border_item.clone()),
+                diagonal: None,
+            };
+            assert_eq!(style.border, expected_border);
+        }
+        let row = 9;
+        for column in 6..9 {
+            let style = model.get_cell_style(0, row, column).unwrap();
+            let expected_border = Border {
+                diagonal_up: false,
+                diagonal_down: false,
+                left: None,
+                right: None,
+                top: None,
+                bottom: None,
+                diagonal: None,
+            };
+            assert_eq!(style.border, expected_border);
+        }
+    }
+    {
+        let column = 5;
+        for row in 5..9 {
+            let style = model.get_cell_style(0, row, column).unwrap();
+            let expected_border = Border {
+                diagonal_up: false,
+                diagonal_down: false,
+                left: None,
+                right: None,
+                top: None,
+                bottom: None,
+                diagonal: None,
+            };
+            assert_eq!(style.border, expected_border);
+        }
+        let column = 9;
+        for row in 5..9 {
+            let style = model.get_cell_style(0, row, column).unwrap();
+            let expected_border = Border {
+                diagonal_up: false,
+                diagonal_down: false,
+                left: None,
+                right: None,
+                top: None,
+                bottom: None,
+                diagonal: None,
+            };
+            assert_eq!(style.border, expected_border);
+        }
+    }
 }
 
 #[test]

base/src/test/user_model/test_paste_csv.rs

@@ -48,6 +48,54 @@ fn tsv_crlf_paste() {
     );
 }
 
+#[test]
+fn cut_paste() {
+    let mut model = UserModel::new_empty("model", "en", "UTC").unwrap();
+    model.set_user_input(0, 1, 1, "42").unwrap();
+    model.set_user_input(0, 1, 2, "=A1*3+1").unwrap();
+
+    // set A1 bold
+    let range = Area {
+        sheet: 0,
+        row: 1,
+        column: 1,
+        width: 1,
+        height: 1,
+    };
+    model.update_range_style(&range, "font.b", "true").unwrap();
+
+    model
+        .set_user_input(0, 2, 1, "A season of faith, \"perfection\"")
+        .unwrap();
+
+    // Select A1:B2 and copy
+    model.set_selected_range(1, 1, 2, 2).unwrap();
+    let copy = model.copy_to_clipboard().unwrap();
+
+    model.set_selected_cell(4, 4).unwrap();
+
+    // paste in cell D4 (4, 4)
+    model
+        .paste_from_clipboard((1, 1, 2, 2), &copy.data, true)
+        .unwrap();
+
+    assert_eq!(model.get_cell_content(0, 4, 4), Ok("42".to_string()));
+    assert_eq!(model.get_cell_content(0, 4, 5), Ok("=D4*3+1".to_string()));
+    assert_eq!(
+        model.get_formatted_cell_value(0, 4, 5),
+        Ok("127".to_string())
+    );
+    // cell D4 must be bold
+    let style_d4 = model.get_cell_style(0, 4, 4).unwrap();
+    assert!(style_d4.font.b);
+
+    // range A1:B2 must be empty
+    assert_eq!(model.get_cell_content(0, 1, 1), Ok("".to_string()));
+    assert_eq!(model.get_cell_content(0, 1, 2), Ok("".to_string()));
+    assert_eq!(model.get_cell_content(0, 2, 1), Ok("".to_string()));
+    assert_eq!(model.get_cell_content(0, 2, 2), Ok("".to_string()));
+}
+
 #[test]
 fn copy_paste_internal() {
     let mut model = UserModel::new_empty("model", "en", "UTC").unwrap();

base/src/user_model/common.rs

@@ -7,7 +7,7 @@ use csv_sniffer::Sniffer;
 use serde::{Deserialize, Serialize};
 
 use crate::{
-    constants::{self, DEFAULT_ROW_HEIGHT},
+    constants::{self, DEFAULT_ROW_HEIGHT, LAST_COLUMN, LAST_ROW},
     expressions::{
         types::{Area, CellReferenceIndex},
         utils::{is_valid_column_number, is_valid_row},
@@ -41,7 +41,7 @@ pub struct Clipboard {
     pub(crate) range: (i32, i32, i32, i32),
 }
 
-#[derive(Serialize, Deserialize)]
+#[derive(Serialize, Deserialize, PartialEq)]
 pub enum BorderType {
     All,
     Inner,
@@ -796,7 +796,6 @@ impl UserModel {
         range: &Area,
         border_area: &BorderArea,
     ) -> Result<(), String> {
-        // FIXME: We need to set the border also in neighbouring cells.
         let sheet = range.sheet;
         let mut diff_list = Vec::new();
         let last_row = range.row + range.height - 1;
@@ -806,12 +805,6 @@ impl UserModel {
                 let old_value = self.model.get_style_for_cell(sheet, row, column)?;
                 let mut style = old_value.clone();
 
-                // First remove all existing borders
-                style.border.top = None;
-                style.border.right = None;
-                style.border.bottom = None;
-                style.border.left = None;
-
                 match border_area.r#type {
                     BorderType::All => {
                         style.border.top = Some(border_area.item.clone());
@@ -868,7 +861,10 @@ impl UserModel {
                         }
                     }
                     BorderType::None => {
-                        // noop, we already removed all the borders
+                        style.border.top = None;
+                        style.border.right = None;
+                        style.border.bottom = None;
+                        style.border.left = None;
                     }
                 }
 
@@ -882,6 +878,122 @@ impl UserModel {
                 });
             }
         }
+        // bottom of the cells above the first
+        if range.row > 1
+            && [
+                BorderType::Top,
+                BorderType::All,
+                BorderType::None,
+                BorderType::Outer,
+            ]
+            .contains(&border_area.r#type)
+        {
+            let row = range.row - 1;
+            for column in range.column..=last_column {
+                let old_value = self.model.get_style_for_cell(sheet, row, column)?;
+                let mut style = old_value.clone();
+                if border_area.r#type == BorderType::None {
+                    style.border.bottom = None;
+                } else {
+                    style.border.bottom = Some(border_area.item.clone());
+                }
+                self.model.set_cell_style(sheet, row, column, &style)?;
+                diff_list.push(Diff::SetCellStyle {
+                    sheet,
+                    row,
+                    column,
+                    old_value: Box::new(old_value),
+                    new_value: Box::new(style),
+                });
+            }
+        }
+        // Cells to the right
+        if last_column < LAST_COLUMN
+            && [
+                BorderType::Right,
+                BorderType::All,
+                BorderType::None,
+                BorderType::Outer,
+            ]
+            .contains(&border_area.r#type)
+        {
+            let column = last_column + 1;
+            for row in range.row..=last_row {
+                let old_value = self.model.get_style_for_cell(sheet, row, column)?;
+                let mut style = old_value.clone();
+                if border_area.r#type == BorderType::None {
+                    style.border.left = None;
+                } else {
+                    style.border.left = Some(border_area.item.clone());
+                }
+                self.model.set_cell_style(sheet, row, column, &style)?;
+                diff_list.push(Diff::SetCellStyle {
+                    sheet,
+                    row,
+                    column,
+                    old_value: Box::new(old_value),
+                    new_value: Box::new(style),
+                });
+            }
+        }
+        // Cells bellow
+        if last_row < LAST_ROW
+            && [
+                BorderType::Bottom,
+                BorderType::All,
+                BorderType::None,
+                BorderType::Outer,
+            ]
+            .contains(&border_area.r#type)
+        {
+            let row = last_row + 1;
+            for column in range.column..=last_column {
+                let old_value = self.model.get_style_for_cell(sheet, row, column)?;
+                let mut style = old_value.clone();
+                if border_area.r#type == BorderType::None {
+                    style.border.top = None;
+                } else {
+                    style.border.top = Some(border_area.item.clone());
+                }
+                self.model.set_cell_style(sheet, row, column, &style)?;
+                diff_list.push(Diff::SetCellStyle {
+                    sheet,
+                    row,
+                    column,
+                    old_value: Box::new(old_value),
+                    new_value: Box::new(style),
+                });
+            }
+        }
+        // Cells to the left
+        if range.column > 1
+            && [
+                BorderType::Left,
+                BorderType::All,
+                BorderType::None,
+                BorderType::Outer,
+            ]
+            .contains(&border_area.r#type)
+        {
+            let column = range.column - 1;
+            for row in range.row..=last_row {
+                let old_value = self.model.get_style_for_cell(sheet, row, column)?;
+                let mut style = old_value.clone();
+                if border_area.r#type == BorderType::None {
+                    style.border.right = None;
+                } else {
+                    style.border.right = Some(border_area.item.clone());
+                }
+                self.model.set_cell_style(sheet, row, column, &style)?;
+                diff_list.push(Diff::SetCellStyle {
+                    sheet,
+                    row,
+                    column,
+                    old_value: Box::new(old_value),
+                    new_value: Box::new(style),
+                });
+            }
+        }
 
         self.push_diff_list(diff_list);
         Ok(())

webapp/src/components/borderPicker.tsx

@@ -7,7 +7,7 @@ import {
   PencilLine,
 } from "lucide-react";
 import type React from "react";
-import { useRef, useState } from "react";
+import { useEffect, useRef, useState } from "react";
 import { useTranslation } from "react-i18next";
 import {
   BorderBottomIcon,
@@ -27,6 +27,7 @@ import ColorPicker from "./colorPicker";
 type BorderPickerProps = {
   className?: string;
   onChange: (border: BorderOptions) => void;
+  onClose: () => void;
   anchorEl: React.RefObject<HTMLElement>;
   anchorOrigin?: PopoverOrigin;
   transformOrigin?: PopoverOrigin;
@@ -36,25 +37,33 @@ type BorderPickerProps = {
 const BorderPicker = (properties: BorderPickerProps) => {
   const { t } = useTranslation();
 
-  const [borderSelected, setBorderSelected] = useState(BorderType.None);
+  const [borderSelected, setBorderSelected] = useState<BorderType | null>(null);
   const [borderColor, setBorderColor] = useState("#000000");
   const [borderStyle, setBorderStyle] = useState(BorderStyle.Thin);
   const [colorPickerOpen, setColorPickerOpen] = useState(false);
   const [stylePickerOpen, setStylePickerOpen] = useState(false);
-  const closePicker = (): void => {
+
+  // biome-ignore lint/correctness/useExhaustiveDependencies: <explanation>
+  useEffect(() => {
+    if (!borderSelected) {
+      return;
+    }
     properties.onChange({
       color: borderColor,
       style: borderStyle,
       border: borderSelected,
     });
-  };
+  }, [borderColor, borderStyle, borderSelected]);
+
+  const onClose = properties.onClose;
+
   const borderColorButton = useRef(null);
   const borderStyleButton = useRef(null);
   return (
     <>
       <StyledPopover
         open={properties.open}
-        onClose={(): void => closePicker()}
+        onClose={onClose}
         anchorEl={properties.anchorEl.current}
         anchorOrigin={
           properties.anchorOrigin || { vertical: "bottom", horizontal: "left" }
@@ -71,13 +80,13 @@ const BorderPicker = (properties: BorderPickerProps) => {
                 $pressed={borderSelected === BorderType.All}
                 onClick={() => {
                   if (borderSelected === BorderType.All) {
-                    setBorderSelected(BorderType.None);
+                    setBorderSelected(null);
                   } else {
                     setBorderSelected(BorderType.All);
                   }
                 }}
                 disabled={false}
-                title={t("workbook.toolbar.borders_button_title")}
+                title={t("toolbar.borders.all")}
               >
                 <BorderAllIcon />
               </Button>
@@ -86,13 +95,13 @@ const BorderPicker = (properties: BorderPickerProps) => {
                 $pressed={borderSelected === BorderType.Inner}
                 onClick={() => {
                   if (borderSelected === BorderType.Inner) {
-                    setBorderSelected(BorderType.None);
+                    setBorderSelected(null);
                   } else {
                     setBorderSelected(BorderType.Inner);
                   }
                 }}
                 disabled={false}
-                title={t("workbook.toolbar.borders_button_title")}
+                title={t("toolbar.borders.inner")}
               >
                 <BorderInnerIcon />
               </Button>
@@ -101,13 +110,13 @@ const BorderPicker = (properties: BorderPickerProps) => {
                 $pressed={borderSelected === BorderType.CenterH}
                 onClick={() => {
                   if (borderSelected === BorderType.CenterH) {
-                    setBorderSelected(BorderType.None);
+                    setBorderSelected(null);
                   } else {
                     setBorderSelected(BorderType.CenterH);
                   }
                 }}
                 disabled={false}
-                title={t("workbook.toolbar.borders_button_title")}
+                title={t("toolbar.borders.horizontal")}
               >
                 <BorderCenterHIcon />
               </Button>
@@ -116,13 +125,13 @@ const BorderPicker = (properties: BorderPickerProps) => {
                 $pressed={borderSelected === BorderType.CenterV}
                 onClick={() => {
                   if (borderSelected === BorderType.CenterV) {
-                    setBorderSelected(BorderType.None);
+                    setBorderSelected(null);
                   } else {
                     setBorderSelected(BorderType.CenterV);
                   }
                 }}
                 disabled={false}
-                title={t("workbook.toolbar.borders_button_title")}
+                title={t("toolbar.borders.vertical")}
               >
                 <BorderCenterVIcon />
               </Button>
@@ -137,7 +146,7 @@ const BorderPicker = (properties: BorderPickerProps) => {
                   }
                 }}
                 disabled={false}
-                title={t("workbook.toolbar.borders_button_title")}
+                title={t("toolbar.borders.outer")}
               >
                 <BorderOuterIcon />
               </Button>
@@ -154,7 +163,7 @@ const BorderPicker = (properties: BorderPickerProps) => {
                   }
                 }}
                 disabled={false}
-                title={t("workbook.toolbar.borders_button_title")}
+                title={t("toolbar.borders.clear")}
               >
                 <BorderNoneIcon />
               </Button>
@@ -169,7 +178,7 @@ const BorderPicker = (properties: BorderPickerProps) => {
                   }
                 }}
                 disabled={false}
-                title={t("workbook.toolbar.borders_button_title")}
+                title={t("toolbar.borders.top")}
               >
                 <BorderTopIcon />
               </Button>
@@ -184,7 +193,7 @@ const BorderPicker = (properties: BorderPickerProps) => {
                   }
                 }}
                 disabled={false}
-                title={t("workbook.toolbar.borders_button_title")}
+                title={t("toolbar.borders.right")}
               >
                 <BorderRightIcon />
               </Button>
@@ -199,7 +208,7 @@ const BorderPicker = (properties: BorderPickerProps) => {
                   }
                 }}
                 disabled={false}
-                title={t("workbook.toolbar.borders_button_title")}
+                title={t("toolbar.borders.bottom")}
               >
                 <BorderBottomIcon />
               </Button>
@@ -214,7 +223,7 @@ const BorderPicker = (properties: BorderPickerProps) => {
                   }
                 }}
                 disabled={false}
-                title={t("workbook.toolbar.borders_button_title")}
+                title={t("toolbar.borders.left")}
               >
                 <BorderLeftIcon />
               </Button>
@@ -228,7 +237,7 @@ const BorderPicker = (properties: BorderPickerProps) => {
                 $pressed={false}
                 disabled={false}
                 ref={borderColorButton}
-                title={t("workbook.toolbar.borders_button_title")}
+                title={t("toolbar.borders.color")}
               >
                 <PencilLine />
               </Button>
@@ -243,7 +252,7 @@ const BorderPicker = (properties: BorderPickerProps) => {
                 type="button"
                 $pressed={false}
                 disabled={false}
-                title={t("workbook.toolbar.borders_button_title")}
+                title={t("toolbar.borders.style")}
               >
                 <BorderStyleIcon />
               </Button>

webapp/src/components/toolbar.tsx

@@ -325,7 +325,7 @@ function Toolbar(properties: ToolbarProperties) {
         onClick={() => setBorderPickerOpen(true)}
         ref={borderButton}
         disabled={!canEdit}
-        title={t("toolbar.borders")}
+        title={t("toolbar.borders.title")}
       >
         <Grid2X2 />
       </StyledButton>
@@ -369,6 +369,8 @@ function Toolbar(properties: ToolbarProperties) {
       <BorderPicker
         onChange={(border): void => {
           properties.onBorderChanged(border);
+        }}
+        onClose={() => {
           setBorderPickerOpen(false);
         }}
         anchorEl={borderButton}

webapp/src/locale/en_us.json

@@ -15,7 +15,6 @@
     "format_number": "Format number",
     "font_color": "Font color",
     "fill_color": "Fill color",
-    "borders": "Borders",
     "decimal_places_increase": "Increase decimal places",
     "decimal_places_decrease": "Decrease decimal places",
     "show_hide_grid_lines": "Show/hide grid lines",
@@ -39,6 +38,21 @@
       "currency_gbp_example": "£",
       "date_short_example": "09/24/2024",
       "date_long_example": "Tuesday, September 24, 2024"
+    },
+    "borders": {
+      "title": "Borders",
+      "all": "All borders",
+      "inner": "Inner borders",
+      "outer": "Outer borders",
+      "top": "Top borders",
+      "bottom": "Bottom borders",
+      "clear": "Clear borders",
+      "left": "Left borders",
+      "right": "Right borders",
+      "horizontal": "Horizontal borders",
+      "vertical": "Vertical borders",
+      "color": "Border color",
+      "style": "Border style"
     }
   },
   "num_fmt": {