fix(editor): can not undo and redo of color of edgeless blocks (#12414)

Close [BS-3507](https://linear.app/affine-design/issue/BS-3507/edgeless-text-颜色无法-undoredo)
Close [BS-3426](https://linear.app/affine-design/issue/BS-3426/frame-修改背景色后不能撤销)

This PR fixes the issue where the color change of edgeless blocks could not be undone/redone, including notes, edgeless-text, and frames. It also addresses the problem of a tiny shape being unexpectedly retained on the canvas. The key changes are:
- Removal of `transact` from the `pop` method of edgeless elements.
- Refactoring of `onPickColor` for all edgeless elements and blocks to better control the lifecycle of custom color property changes.
- Addition of the missing custom background color feature for notes.
- Addition of undo/redo color tests for notes, frames, and edgeless-text.

<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
## Summary by CodeRabbit

- **New Features**
  - Added undo and redo support for color changes in frames, notes, and text blocks, allowing users to revert or reapply background and text color modifications.

- **Bug Fixes**
  - Improved reliability of color picker interactions, ensuring consistent state management and transactional updates during color changes.

- **Tests**
  - Introduced new end-to-end tests to verify undo/redo functionality for color changes in frames, notes, and text blocks.

- **Refactor**
  - Streamlined color picker event handling for better maintainability and consistency across toolbars and style panels.
  - Updated style panel structure and event handling for improved interaction and state management.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

blocksuite/affine/blocks/frame/src/frame-toolbar.ts

@@ -160,19 +160,30 @@ const builtinSurfaceToolbarConfig = {
             background => resolveColor(background, theme)
           ) ?? DefaultTheme.transparent;
         const onPick = (e: PickColorEvent) => {
-          if (e.type === 'pick') {
+          switch (e.type) {
-            const color = e.detail.value;
+            case 'pick':
-            for (const model of models) {
+              {
-              const props = packColor(field, color);
+                const color = e.detail.value;
-              ctx.std
+                const props = packColor(field, color);
-                .get(EdgelessCRUDIdentifier)
+                const crud = ctx.std.get(EdgelessCRUDIdentifier);
-                .updateElement(model.id, props);
+                models.forEach(model => {
-            }
+                  crud.updateElement(model.id, props);
-            return;
+                });
-          }
+              }
-
+              break;
-          for (const model of models) {
+            case 'start':
-            model[e.type === 'start' ? 'stash' : 'pop'](field);
+              ctx.store.captureSync();
+              models.forEach(model => {
+                model.stash(field);
+              });
+              break;
+            case 'end':
+              ctx.store.transact(() => {
+                models.forEach(model => {
+                  model.pop(field);
+                });
+              });
+              break;
           }
         };
 

blocksuite/affine/blocks/note/src/components/edgeless-note-style-panel.ts

@@ -40,12 +40,12 @@ export class EdgelessNoteStylePanel extends SignalWatcher(
   @property({ attribute: false })
   accessor std!: BlockStdScope;
 
-  @query('.edgeless-note-style-panel')
-  private accessor _panel!: HTMLDivElement;
-
   @state()
   accessor tabType: 'style' | 'customColor' = 'style';
 
+  @query('div.edgeless-note-style-panel-container')
+  accessor container!: HTMLDivElement;
+
   static override styles = css`
     .edgeless-note-style-panel {
       display: flex;
@@ -187,7 +187,32 @@ export class EdgelessNoteStylePanel extends SignalWatcher(
   };
 
   private readonly _pickColor = (e: PickColorEvent) => {
-    console.log(e);
+    switch (e.type) {
+      case 'pick':
+        {
+          const color = e.detail.value;
+          const crud = this.std.get(EdgelessCRUDIdentifier);
+          this.notes.forEach(note => {
+            crud.updateElement(note.id, {
+              background: color,
+            } satisfies Partial<NoteProps>);
+          });
+        }
+        break;
+      case 'start':
+        this._beforeChange();
+        this.notes.forEach(note => {
+          note.stash('background');
+        });
+        break;
+      case 'end':
+        this.std.store.transact(() => {
+          this.notes.forEach(note => {
+            note.pop('background');
+          });
+        });
+        break;
+    }
   };
 
   private readonly _selectShadow = (e: CustomEvent<NoteShadow>) => {
@@ -265,7 +290,7 @@ export class EdgelessNoteStylePanel extends SignalWatcher(
   };
 
   private _renderStylePanel() {
-    return html` <div class="edgeless-note-style-panel">
+    return html`<div class="edgeless-note-style-panel">
       <div class="edgeless-note-style-section">
         <div class="edgeless-note-style-section-title">Fill color</div>
         <edgeless-color-panel
@@ -369,9 +394,11 @@ export class EdgelessNoteStylePanel extends SignalWatcher(
   }
 
   override firstUpdated() {
-    this.disposables.addFromEvent(this._panel, 'click', e => {
+    if (this.container) {
-      e.stopPropagation();
+      this.disposables.addFromEvent(this.container, 'click', e => {
-    });
+        e.stopPropagation();
+      });
+    }
   }
 
   override render() {
@@ -383,11 +410,18 @@ export class EdgelessNoteStylePanel extends SignalWatcher(
             ${PaletteIcon()}
           </editor-icon-button>
         `}
+        @toggle=${(e: CustomEvent<boolean>) => {
+          if (!e.detail) {
+            this.tabType = 'style';
+          }
+        }}
       >
-        ${choose(this.tabType, [
+        <div class="edgeless-note-style-panel-container">
-          ['style', () => this._renderStylePanel()],
+          ${choose(this.tabType, [
-          ['customColor', () => this._renderCustomColorPanel()],
+            ['style', () => this._renderStylePanel()],
-        ])}
+            ['customColor', () => this._renderCustomColorPanel()],
+          ])}
+        </div>
       </editor-menu-button>
     `;
   }

blocksuite/affine/gfx/brush/src/toolbar/configs/brush.ts

@@ -68,19 +68,30 @@ export const brushToolbarConfig = {
             resolveColor(color, theme)
           ) ?? resolveColor(DefaultTheme.black, theme);
         const onPick = (e: PickColorEvent) => {
-          if (e.type === 'pick') {
+          switch (e.type) {
-            const color = e.detail.value;
+            case 'pick':
-            for (const model of models) {
+              {
-              const props = packColor(field, color);
+                const color = e.detail.value;
-              ctx.std
+                const props = packColor(field, color);
-                .get(EdgelessCRUDIdentifier)
+                const crud = ctx.std.get(EdgelessCRUDIdentifier);
-                .updateElement(model.id, props);
+                models.forEach(model => {
-            }
+                  crud.updateElement(model.id, props);
-            return;
+                });
-          }
+              }
-
+              break;
-          for (const model of models) {
+            case 'start':
-            model[e.type === 'start' ? 'stash' : 'pop'](field);
+              ctx.store.captureSync();
+              models.forEach(model => {
+                model.stash(field);
+              });
+              break;
+            case 'end':
+              ctx.store.transact(() => {
+                models.forEach(model => {
+                  model.pop(field);
+                });
+              });
+              break;
           }
         };
 

blocksuite/affine/gfx/connector/src/toolbar/config.ts

@@ -148,19 +148,30 @@ export const connectorToolbarConfig = {
           ) ?? resolveColor(DefaultTheme.connectorColor, theme);
 
         const onPickColor = (e: PickColorEvent) => {
-          if (e.type === 'pick') {
+          switch (e.type) {
-            const color = e.detail.value;
+            case 'pick':
-            for (const model of models) {
+              {
-              const props = packColor(field, color);
+                const color = e.detail.value;
-              ctx.std
+                const props = packColor(field, color);
-                .get(EdgelessCRUDIdentifier)
+                const crud = ctx.std.get(EdgelessCRUDIdentifier);
-                .updateElement(model.id, props);
+                models.forEach(model => {
-            }
+                  crud.updateElement(model.id, props);
-            return;
+                });
-          }
+              }
-
+              break;
-          for (const model of models) {
+            case 'start':
-            model[e.type === 'start' ? 'stash' : 'pop'](field);
+              ctx.store.captureSync();
+              models.forEach(model => {
+                model.stash(field);
+              });
+              break;
+            case 'end':
+              ctx.store.transact(() => {
+                models.forEach(model => {
+                  model.pop(field);
+                });
+              });
+              break;
           }
         };
 

blocksuite/affine/gfx/shape/src/toolbar/config.ts

@@ -15,6 +15,7 @@ import {
   isTransparent,
   LineWidth,
   MindmapElementModel,
+  type Palette,
   resolveColor,
   ShapeElementModel,
   type ShapeName,
@@ -167,56 +168,53 @@ export const shapeToolbarConfig = {
         const strokeStyle =
           getMostCommonValue(mapped, 'strokeStyle') ?? StrokeStyle.Solid;
 
-        const onPickFillColor = (e: CustomEvent<PickColorEvent>) => {
+        const pickColorWrapper =
-          e.stopPropagation();
+          (field: string, pickCallback: (palette: Palette) => void) =>
+          (e: CustomEvent<PickColorEvent>) => {
+            e.stopPropagation();
 
-          const d = e.detail;
+            switch (e.detail.type) {
-
+              case 'pick':
-          const field = 'fillColor';
+                pickCallback(e.detail.detail);
-
+                break;
-          if (d.type === 'pick') {
+              case 'start':
-            const value = d.detail.value;
+                ctx.store.captureSync();
-            const filled = isTransparent(value);
+                models.forEach(model => {
-            for (const model of models) {
+                  model.stash(field);
-              const props = packColor(field, value);
+                });
-              // If `filled` can be set separately, this logic can be removed
+                break;
-              if (field && !model.filled) {
+              case 'end':
-                const color = getTextColor(value, filled);
+                ctx.store.transact(() => {
-                Object.assign(props, { filled, color });
+                  models.forEach(model => {
-              }
+                    model.pop(field);
-              ctx.std
+                  });
-                .get(EdgelessCRUDIdentifier)
+                });
-                .updateElement(model.id, props);
             }
-            return;
+          };
-          }
 
-          for (const model of models) {
+        const onPickFillColor = pickColorWrapper('fillColor', palette => {
-            model[d.type === 'start' ? 'stash' : 'pop'](field);
+          const value = palette.value;
-          }
+          const filled = isTransparent(value);
-        };
+          const props = packColor('fillColor', value);
-        const onPickStrokeColor = (e: CustomEvent<PickColorEvent>) => {
+          const crud = ctx.std.get(EdgelessCRUDIdentifier);
-          e.stopPropagation();
+          models.forEach(model => {
-
+            if (filled && !model.filled) {
-          const d = e.detail;
+              const color = getTextColor(value, filled);
-
+              Object.assign(props, { filled, color });
-          const field = 'strokeColor';
-
-          if (d.type === 'pick') {
-            const value = d.detail.value;
-            for (const model of models) {
-              const props = packColor(field, value);
-              ctx.std
-                .get(EdgelessCRUDIdentifier)
-                .updateElement(model.id, props);
             }
-            return;
+            crud.updateElement(model.id, props);
-          }
+          });
+        });
+
+        const onPickStrokeColor = pickColorWrapper('strokeColor', palette => {
+          const value = palette.value;
+          const props = packColor('strokeColor', value);
+          const crud = ctx.std.get(EdgelessCRUDIdentifier);
+          models.forEach(model => {
+            crud.updateElement(model.id, props);
+          });
+        });
 
-          for (const model of models) {
-            model[d.type === 'start' ? 'stash' : 'pop'](field);
-          }
-        };
         const onPickStrokeStyle = (e: CustomEvent<LineDetailType>) => {
           e.stopPropagation();
 

blocksuite/affine/gfx/text/src/toolbar/actions.ts

@@ -210,17 +210,29 @@ export function createTextActions<
           ) ?? resolveColor(defaultColor, theme);
 
         const onPick = (e: PickColorEvent) => {
-          if (e.type === 'pick') {
+          switch (e.type) {
-            const color = e.detail.value;
+            case 'pick':
-            for (const model of models) {
+              {
-              const props = packColor(field, color);
+                const color = e.detail.value;
-              update(ctx, model, props);
+                const props = packColor(field, color);
-            }
+                models.forEach(model => {
-            return;
+                  update(ctx, model, props);
-          }
+                });
-
+              }
-          for (const model of models) {
+              break;
-            stash(model, e.type === 'start' ? 'stash' : 'pop', field);
+            case 'start':
+              ctx.store.captureSync();
+              models.forEach(model => {
+                stash(model, 'stash', field);
+              });
+              break;
+            case 'end':
+              ctx.store.transact(() => {
+                models.forEach(model => {
+                  stash(model, 'pop', field);
+                });
+              });
+              break;
           }
         };
 

blocksuite/framework/std/src/gfx/model/surface/element-model.ts

@@ -278,9 +278,7 @@ export abstract class GfxPrimitiveElementModel<
 
     if (getFieldPropsSet(this).has(prop as string)) {
       if (!isEqual(value, this.yMap.get(prop as string))) {
-        this.surface.store.transact(() => {
+        this.yMap.set(prop as string, value);
-          this.yMap.set(prop as string, value);
-        });
       }
     } else {
       console.warn('pop a prop that is not field or local:', prop);

tests/affine-desktop/e2e/workspace.spec.ts

@@ -2,7 +2,10 @@ import path from 'node:path';
 
 import type { apis } from '@affine/electron-api';
 import { test } from '@affine-test/kit/electron';
-import { getBlockSuiteEditorTitle } from '@affine-test/kit/utils/page-logic';
+import {
+  getBlockSuiteEditorTitle,
+  waitForEditorLoad,
+} from '@affine-test/kit/utils/page-logic';
 import {
   clickNewPageButton,
   clickSideBarCurrentWorkspaceBanner,
@@ -99,8 +102,8 @@ test('export then add', async ({ page, appInfo, workspace }) => {
   await page.waitForTimeout(1000);
 
   // find button which has the title "test1"
-  await page.getByText('test1').click();
+  await page.getByTestId('page-list-item').getByText('test1').click();
-
+  await waitForEditorLoad(page);
   const title = page.locator('[data-block-is-title] >> text="test1"');
   await expect(title).toBeVisible();
 });

tests/blocksuite/e2e/edgeless/edgeless-text.spec.ts

@@ -6,7 +6,9 @@ import {
   autoFit,
   captureHistory,
   cutByKeyboard,
+  dblclickView,
   dragBetweenIndices,
+  edgelessCommonSetup,
   enterPlaygroundRoom,
   getEdgelessSelectedRect,
   getPageSnapshot,
@@ -19,6 +21,7 @@ import {
   pressBackspace,
   pressEnter,
   pressEscape,
+  redoByKeyboard,
   selectAllByKeyboard,
   setEdgelessTool,
   switchEditorMode,
@@ -598,3 +601,62 @@ test('press backspace at the start of first line when edgeless text exist', asyn
     `${testInfo.title}_finial.json`
   );
 });
+
+test('undo/redo should work when changing text color', async ({ page }) => {
+  await edgelessCommonSetup(page);
+  await dblclickView(page, [100, 100]);
+  await type(page, 'abc');
+  await pressEscape(page, 3);
+  await waitNextFrame(page);
+
+  const edgelessText = page.locator('affine-edgeless-text');
+  await edgelessText.click();
+
+  const getTextColor = async () => {
+    return edgelessText.locator('span[data-v-text="true"]').evaluate(el => {
+      return getComputedStyle(el).color;
+    });
+  };
+  const colorPanel = page.locator('edgeless-color-picker-button');
+
+  let prevTextColor = await getTextColor();
+
+  // preset color
+  {
+    await colorPanel.click();
+    await colorPanel.getByLabel('LightRed').click();
+    expect(await getTextColor()).not.toBe(prevTextColor);
+
+    await undoByKeyboard(page);
+    await waitNextFrame(page);
+    expect(await getTextColor()).toBe(prevTextColor);
+
+    await redoByKeyboard(page);
+    await waitNextFrame(page);
+    expect(await getTextColor()).not.toBe(prevTextColor);
+  }
+
+  prevTextColor = await getTextColor();
+
+  // custom color
+  {
+    await colorPanel.click();
+    await colorPanel.locator('edgeless-color-custom-button').click();
+    await page.locator('.color-palette').click({
+      position: {
+        x: 100,
+        y: 100,
+      },
+    });
+    await pressEscape(page);
+
+    expect(await getTextColor()).not.toBe(prevTextColor);
+    await undoByKeyboard(page);
+    await waitNextFrame(page);
+    expect(await getTextColor()).toBe(prevTextColor);
+
+    await redoByKeyboard(page);
+    await waitNextFrame(page);
+    expect(await getTextColor()).not.toBe(prevTextColor);
+  }
+});

tests/blocksuite/e2e/edgeless/frame/frame.spec.ts

@@ -24,9 +24,11 @@ import {
 import {
   pressBackspace,
   pressEscape,
+  redoByKeyboard,
   SHORT_KEY,
   undoByKeyboard,
 } from '../../utils/actions/keyboard.js';
+import { waitNextFrame } from '../../utils/actions/misc.js';
 import {
   assertCanvasElementsCount,
   assertContainerChildCount,
@@ -420,3 +422,59 @@ test('undo should work when create a frame by dragging', async ({ page }) => {
   await undoByKeyboard(page);
   await expect(page.locator('affine-frame')).toHaveCount(0);
 });
+
+test('undo/redo should work when change frame background', async ({ page }) => {
+  await createFrame(page, [50, 50], [450, 450]);
+  await pressEscape(page);
+
+  const frameTitle = page.locator('affine-frame-title');
+  await frameTitle.click();
+
+  const getFrameBackground = async () => {
+    return page.locator('affine-frame .affine-frame-container').evaluate(el => {
+      return getComputedStyle(el).backgroundColor;
+    });
+  };
+  const colorPanel = page.locator('edgeless-color-picker-button');
+
+  let prevBackground = await getFrameBackground();
+
+  // preset color
+  {
+    await colorPanel.click();
+    await colorPanel.getByLabel('LightRed').click();
+    expect(await getFrameBackground()).not.toBe(prevBackground);
+
+    await undoByKeyboard(page);
+    await waitNextFrame(page);
+    expect(await getFrameBackground()).toBe(prevBackground);
+
+    await redoByKeyboard(page);
+    await waitNextFrame(page);
+    expect(await getFrameBackground()).not.toBe(prevBackground);
+  }
+
+  prevBackground = await getFrameBackground();
+
+  // custom color
+  {
+    await colorPanel.click();
+    await colorPanel.locator('edgeless-color-custom-button').click();
+    await page.locator('.color-palette').click({
+      position: {
+        x: 100,
+        y: 100,
+      },
+    });
+    await pressEscape(page);
+
+    expect(await getFrameBackground()).not.toBe(prevBackground);
+    await undoByKeyboard(page);
+    await waitNextFrame(page);
+    expect(await getFrameBackground()).toBe(prevBackground);
+
+    await redoByKeyboard(page);
+    await waitNextFrame(page);
+    expect(await getFrameBackground()).not.toBe(prevBackground);
+  }
+});

tests/blocksuite/e2e/edgeless/note/undo-redo.spec.ts

@@ -13,6 +13,7 @@ import {
   initEmptyEdgelessState,
   initSixParagraphs,
   pasteByKeyboard,
+  pressEscape,
   redoByClick,
   redoByKeyboard,
   selectNoteInEdgeless,
@@ -137,3 +138,63 @@ test('continuous undo and redo (note block add operation) should work', async ({
   count = await countBlock(page, 'affine-edgeless-note');
   expect(count).toBe(4);
 });
+
+test('undo/redo should work when change note custom background', async ({
+  page,
+}) => {
+  await enterPlaygroundRoom(page);
+  const { noteId } = await initEmptyEdgelessState(page);
+  await switchEditorMode(page);
+  await selectNoteInEdgeless(page, noteId);
+
+  const getNoteBackground = async () => {
+    return page.locator('edgeless-note-background > div').evaluate(el => {
+      return getComputedStyle(el).backgroundColor;
+    });
+  };
+
+  const stylePanel = page.locator('edgeless-note-style-panel');
+
+  let prevBackground = await getNoteBackground();
+
+  // preset color
+  {
+    await stylePanel.click();
+    await stylePanel.getByLabel('Red').click();
+    await pressEscape(page);
+
+    expect(await getNoteBackground()).not.toBe(prevBackground);
+    await undoByKeyboard(page);
+    await waitNextFrame(page);
+    expect(await getNoteBackground()).toBe(prevBackground);
+
+    await redoByKeyboard(page);
+    await waitNextFrame(page);
+    expect(await getNoteBackground()).not.toBe(prevBackground);
+  }
+
+  prevBackground = await getNoteBackground();
+
+  // custom color
+  {
+    await selectNoteInEdgeless(page, noteId);
+    await stylePanel.click();
+    await stylePanel.locator('edgeless-color-custom-button').click();
+    await stylePanel.locator('.color-palette').click({
+      position: {
+        x: 100,
+        y: 100,
+      },
+    });
+    await pressEscape(page);
+
+    expect(await getNoteBackground()).not.toBe(prevBackground);
+    await undoByKeyboard(page);
+    await waitNextFrame(page);
+    expect(await getNoteBackground()).toBe(prevBackground);
+
+    await redoByKeyboard(page);
+    await waitNextFrame(page);
+    expect(await getNoteBackground()).not.toBe(prevBackground);
+  }
+});