-
Notifications
You must be signed in to change notification settings - Fork 69
Master add shortcuts matho #7536
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,9 +24,11 @@ import { resetTimeoutDuration } from "../../src/components/helpers/touch_scroll_ | |
| import { PaintFormatStore } from "../../src/components/paint_format_button/paint_format_store"; | ||
| import { CellPopoverStore } from "../../src/components/popover"; | ||
| import { buildSheetLink, toCartesian, toZone, zoneToXc } from "../../src/helpers"; | ||
| import { handleCopyPasteResult } from "../../src/helpers/ui/paste_interactive"; | ||
| import { Store } from "../../src/store_engine"; | ||
| import { ClientFocusStore } from "../../src/stores/client_focus_store"; | ||
| import { HighlightStore } from "../../src/stores/highlight_store"; | ||
| import { NotificationStore } from "../../src/stores/notification_store"; | ||
| import { Align, ClipboardMIMEType } from "../../src/types"; | ||
| import { FileStore } from "../__mocks__/mock_file_store"; | ||
| import { MockTransportService } from "../__mocks__/transport_service"; | ||
|
|
@@ -127,6 +129,14 @@ let composerFocusStore: Store<ComposerFocusStore>; | |
|
|
||
| jest.useFakeTimers(); | ||
| mockChart(); | ||
| jest.mock("../../src/actions/menu_items_actions.ts", () => { | ||
| const originalModule = jest.requireActual("../../src/actions/menu_items_actions.ts"); | ||
| return { | ||
| __esModule: true, | ||
| ...originalModule, | ||
| INSERT_TABLE: jest.fn(originalModule.INSERT_TABLE), | ||
| }; | ||
| }); | ||
|
|
||
| describe("Grid component", () => { | ||
| beforeEach(async () => { | ||
|
|
@@ -942,6 +952,14 @@ describe("Grid component", () => { | |
| expect(model.getters.getActiveSheetId()).toBe("third"); | ||
| }); | ||
|
|
||
| test("Pressing Shift+F11 insert a new sheet", () => { | ||
| expect(model.getters.getSheetIds()).toHaveLength(1); | ||
| keyDown({ key: "F11", shiftKey: true }); | ||
| const sheetIds = model.getters.getSheetIds(); | ||
| expect(sheetIds).toHaveLength(2); | ||
| expect(model.getters.getActiveSheetId()).toBe(sheetIds[1]); | ||
| }); | ||
|
|
||
| test("pressing Ctrl+K opens the link editor", async () => { | ||
| await keyDown({ key: "k", ctrlKey: true }); | ||
| expect(fixture.querySelector(".o-link-editor")).not.toBeNull(); | ||
|
|
@@ -1791,7 +1809,7 @@ describe("Copy paste keyboard shortcut", () => { | |
| const fileStore = new FileStore(); | ||
| beforeEach(async () => { | ||
| clipboardData = new MockClipboardData(); | ||
| ({ parent, model, fixture } = await mountSpreadsheet({ | ||
| ({ parent, model, fixture, env } = await mountSpreadsheet({ | ||
| model: new Model({}, { external: { fileStore } }), | ||
| })); | ||
| sheetId = model.getters.getActiveSheetId(); | ||
|
|
@@ -1953,27 +1971,61 @@ describe("Copy paste keyboard shortcut", () => { | |
| setCellContent(model, "B2", "b2"); | ||
| selectCell(model, "B2"); | ||
| keyDown({ key: "D", ctrlKey: true }); | ||
| expect(model.dispatch("COPY_PASTE_CELLS_ABOVE")).toBeSuccessfullyDispatched(); | ||
| expect(getCell(model, "B2")?.content).toBe("b1"); | ||
|
|
||
| setCellContent(model, "C1", "c1"); | ||
| setCellContent(model, "D1", "d1"); | ||
| setSelection(model, ["B2:D2"]); | ||
| keyDown({ key: "D", ctrlKey: true }); | ||
| expect(model.dispatch("COPY_PASTE_CELLS_ABOVE")).toBeSuccessfullyDispatched(); | ||
| expect(getCell(model, "B2")?.content).toBe("b1"); | ||
| expect(getCell(model, "C2")?.content).toBe("c1"); | ||
| expect(getCell(model, "D2")?.content).toBe("d1"); | ||
| }); | ||
|
|
||
| test("can copy and paste cell(s) on left using CTRL+R", async () => { | ||
| test("banane", () => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🍌 🍌 🍌 |
||
| setCellContent(model, "A1", "a1"); | ||
| setCellContent(model, "B1", "b1"); | ||
| selectCell(model, "B1"); | ||
| keyDown({ key: "R", ctrlKey: true }); | ||
| expect(model.dispatch("COPY_PASTE_CELLS_ON_LEFT")).toBeSuccessfullyDispatched(); | ||
| merge(model, "A2:A3"); | ||
| setSelection(model, ["A1:A3"]); | ||
| handleCopyPasteResult(env, { type: "COPY_PASTE_CELLS_ON_ZONE" }); | ||
| // @ts-ignore | ||
| const notificationStore = env.__spreadsheet_stores__.get(NotificationStore); | ||
|
Comment on lines
+1990
to
+1991
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid ts-ignore when possible. I'm guessing |
||
| expect(notificationStore.raiseError).toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| test("can copy and paste cell(s) on left using CTRL+R", async () => { | ||
| setCellContent(model, "A2", "a2"); | ||
| setCellContent(model, "B2", "b2"); | ||
| selectCell(model, "B2"); | ||
| keyDown({ key: "R", ctrlKey: true }); | ||
| expect(getCell(model, "B2")?.content).toBe("a2"); | ||
|
|
||
| setCellContent(model, "A3", "a3"); | ||
| setSelection(model, ["B1:B3"]); | ||
| setCellContent(model, "A4", "a4"); | ||
| setSelection(model, ["B2:B4"]); | ||
| keyDown({ key: "R", ctrlKey: true }); | ||
| expect(model.dispatch("COPY_PASTE_CELLS_ON_LEFT")).toBeSuccessfullyDispatched(); | ||
| expect(getCell(model, "B2")?.content).toBe("a2"); | ||
| expect(getCell(model, "B3")?.content).toBe("a3"); | ||
| expect(getCell(model, "B4")?.content).toBe("a4"); | ||
| }); | ||
|
|
||
| test("can copy and paste cell(s) on zone using CTRL+ENTER", async () => { | ||
| setCellContent(model, "A1", "a1"); | ||
| setSelection(model, ["A1:B2"]); | ||
| keyDown({ key: "Enter", ctrlKey: true }); | ||
| expect(getCell(model, "A1")?.content).toBe("a1"); | ||
| expect(getCell(model, "A2")?.content).toBe("a1"); | ||
| expect(getCell(model, "B1")?.content).toBe("a1"); | ||
| expect(getCell(model, "B2")?.content).toBe("a1"); | ||
| }); | ||
|
|
||
| test("Alt+T -> Table", async () => { | ||
| setSelection(model, ["A1:A5"]); | ||
| await keyDown({ key: "T", altKey: true }); | ||
| expect(model.getters.getTable({ sheetId, row: 0, col: 0 })).toMatchObject({ | ||
| range: { zone: toZone("A1:A5") }, | ||
| }); | ||
| const { INSERT_TABLE } = require("../../src/actions/menu_items_actions"); | ||
| expect(INSERT_TABLE as jest.Mock).toHaveBeenCalled(); | ||
|
Comment on lines
+2027
to
+2028
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd remove this (and the In general avoid mocking imports unless absolutely necessary. It makes tests harder to understand, and breaks if we ever move files around. Testing what the result of an action is is the superior option over testing |
||
| }); | ||
|
|
||
| test("Clipboard visible zones (copy) will be cleaned after hitting esc", async () => { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -97,13 +97,15 @@ describe("link display component", () => { | |
| setCellContent(model, "A1", "HELLO"); | ||
| await rightClickCell(model, "A1"); | ||
| expect( | ||
| fixture.querySelector(".o-menu .o-menu-item[data-name='insert_link']")?.textContent | ||
| fixture.querySelector(".o-menu .o-menu-item[data-name='insert_link'] .o-menu-item-name") | ||
| ?.textContent | ||
| ).toBe("Insert link"); | ||
|
Comment on lines
99
to
102
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nitpick: see our custom jest matchers in |
||
|
|
||
| setCellContent(model, "A1", "[label](url.com)"); | ||
| await rightClickCell(model, "A1"); | ||
| expect( | ||
| fixture.querySelector(".o-menu .o-menu-item[data-name='insert_link']")?.textContent | ||
| fixture.querySelector(".o-menu .o-menu-item[data-name='insert_link'] .o-menu-item-name") | ||
| ?.textContent | ||
| ).toBe("Edit link"); | ||
| }); | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
allowDispatch should not change the state of the plugin. and
this.originSheetIdis not used here anywaysame comment for copy on left/copy on zone