Skip to content

Commit

Permalink
Fix #2584: Safari context menu event causes selection to be expanded (#…
Browse files Browse the repository at this point in the history
…2588)

* Fix #2584: Safari context menu event causes selection to be expanded

* fix test
  • Loading branch information
JiuqingSong authored Apr 20, 2024
1 parent 1269238 commit b0cbd9a
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import {
export function editTable(editor: IEditor, operation: TableOperation) {
editor.focus();

fixUpSafariSelection(editor);

formatTableWithContentModel(editor, 'editTable', tableModel => {
switch (operation) {
case 'alignCellLeft':
Expand Down Expand Up @@ -88,3 +90,21 @@ export function editTable(editor: IEditor, operation: TableOperation) {
}
});
}

// In safari, when open context menu under a table, it may expand the range selection to the beginning of next table cell.
// So we make a workaround here to collapse the selection when need, to avoid unexpected table editing behavior
// (e.g. insert two columns but actually need one only)
function fixUpSafariSelection(editor: IEditor) {
if (editor.getEnvironment().isSafari) {
const selection = editor.getDOMSelection();

if (selection?.type == 'range' && !selection.range.collapsed) {
selection.range.collapse(true /*toStart*/);
editor.setDOMSelection({
type: 'range',
range: selection.range,
isReverted: false,
});
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ describe('editTable', () => {
let editor: IEditor;
let focusSpy: jasmine.Spy;
let formatTableWithContentModelSpy: jasmine.Spy;
let getEnvironmentSpy: jasmine.Spy;
let getDOMSelectionSpy: jasmine.Spy;
let setDOMSelectionSpy: jasmine.Spy;
const mockedTable = 'TABLE' as any;

function runTest(operation: TableOperation, expectedSpy: jasmine.Spy, ...parameters: string[]) {
Expand All @@ -29,10 +32,15 @@ describe('editTable', () => {
jasmine.anything()
);
expect(expectedSpy).toHaveBeenCalledWith(mockedTable, ...parameters);
expect(getDOMSelectionSpy).not.toHaveBeenCalled();
expect(setDOMSelectionSpy).not.toHaveBeenCalled();
}

beforeEach(() => {
focusSpy = jasmine.createSpy('focus');
getEnvironmentSpy = jasmine.createSpy('getEnvironment').and.returnValue({});
getDOMSelectionSpy = jasmine.createSpy('getDOMSelection');
setDOMSelectionSpy = jasmine.createSpy('setDOMSelection');
formatTableWithContentModelSpy = spyOn(
formatTableWithContentModel,
'formatTableWithContentModel'
Expand All @@ -42,6 +50,9 @@ describe('editTable', () => {

editor = {
focus: focusSpy,
getEnvironment: getEnvironmentSpy,
getDOMSelection: getDOMSelectionSpy,
setDOMSelection: setDOMSelectionSpy,
} as any;
});

Expand Down Expand Up @@ -240,4 +251,37 @@ describe('editTable', () => {
runTest('splitVertically', spy);
});
});

it('edit in safar', () => {
const spy = spyOn(alignTableCell, 'alignTableCellHorizontally');
const collapseSpy = jasmine.createSpy('collapse');
const mockedRange = {
collapsed: false,
collapse: collapseSpy,
};

getEnvironmentSpy.and.returnValue({
isSafari: true,
});
getDOMSelectionSpy.and.returnValue({
type: 'range',
range: mockedRange,
});

editTable(editor, 'alignCellLeft');

expect(formatTableWithContentModelSpy).toHaveBeenCalledWith(
editor,
'editTable',
jasmine.anything()
);
expect(spy).toHaveBeenCalledWith(mockedTable, 'alignCellLeft');
expect(getDOMSelectionSpy).toHaveBeenCalled();
expect(setDOMSelectionSpy).toHaveBeenCalledWith({
type: 'range',
range: mockedRange,
isReverted: false,
});
expect(collapseSpy).toHaveBeenCalledWith(true);
});
});

0 comments on commit b0cbd9a

Please sign in to comment.