Skip to content

Commit

Permalink
[FIX] BottomBarSheet: Rename a sheet with style content on Firefox
Browse files Browse the repository at this point in the history
The issue addressed in PR #3351 unfortunately used a
`contentEditable` value that is not compatible with Firefox [1]

This revision implements the first strategy explored by the
aforementioned PR by removing the full format from  the contenteditable
span when we stop the edition.

Testing required to define some properties as JSdom does not properly
support the implementation of innerText (see [2])

[1]: https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/contenteditable#browser_compatibility
[2]: jsdom/jsdom#1245

Task: 3754944
  • Loading branch information
rrahir committed Mar 25, 2024
1 parent f883740 commit c443de6
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 5 deletions.
7 changes: 5 additions & 2 deletions src/components/bottom_bar_sheet/bottom_bar_sheet.ts
Expand Up @@ -155,13 +155,16 @@ export class BottomBarSheet extends Component<Props, SpreadsheetChildEnv> {
}

private stopEdition() {
if (!this.state.isEditing) return;
const input = this.sheetNameRef.el;
if (!this.state.isEditing || !input) return;

this.state.isEditing = false;
this.editionState = "initializing";
this.sheetNameRef.el?.blur();
input.blur();

const inputValue = this.getInputContent() || "";
input.innerText = inputValue;

interactiveRenameSheet(this.env, this.props.sheetId, inputValue, () => this.startEdition());
}

Expand Down
2 changes: 1 addition & 1 deletion src/components/bottom_bar_sheet/bottom_bar_sheet.xml
Expand Up @@ -19,7 +19,7 @@
t-on-dblclick="() => this.onDblClick()"
t-on-focusout="() => this.onFocusOut()"
t-on-keydown="(ev) => this.onKeyDown(ev)"
t-att-contenteditable="state.isEditing ? 'plaintext-only': 'false'"
t-att-contenteditable="state.isEditing ? 'true': 'false'"
/>
<span class="o-sheet-icon ms-1" t-on-click.stop="(ev) => this.onIconClick(ev)">
<t t-call="o-spreadsheet-Icon.TRIANGLE_DOWN"/>
Expand Down
20 changes: 18 additions & 2 deletions tests/components/bottom_bar.test.ts
Expand Up @@ -258,7 +258,7 @@ describe("BottomBar component", () => {
expect(sheetName.getAttribute("contenteditable")).toEqual("false");
triggerMouseEvent(sheetName, "dblclick");
await nextTick();
expect(sheetName.getAttribute("contenteditable")).toEqual("plaintext-only");
expect(sheetName.getAttribute("contenteditable")).toEqual("true");
expect(document.activeElement).toEqual(sheetName);
});

Expand All @@ -277,7 +277,7 @@ describe("BottomBar component", () => {
await nextTick();
await click(fixture, ".o-menu-item[data-name='rename'");
const sheetName = fixture.querySelector<HTMLElement>(".o-sheet-name")!;
expect(sheetName.getAttribute("contenteditable")).toEqual("plaintext-only");
expect(sheetName.getAttribute("contenteditable")).toEqual("true");
expect(document.activeElement).toEqual(sheetName);
});

Expand Down Expand Up @@ -337,6 +337,22 @@ describe("BottomBar component", () => {
expect(window.getSelection()?.toString()).toEqual("ThisIsASheet");
expect(document.activeElement).toEqual(sheetName);
});

test("Pasting styled content in sheet name and renaming sheet does not throw a trackback", async () => {
const HTML = `<span style="color: rgb(242, 44, 61); background-color: rgb(0, 0, 0);">HELLO</span>`;

const sheetName = fixture.querySelector<HTMLElement>(".o-sheet-name")!;
triggerMouseEvent(sheetName, "dblclick");
await nextTick();

sheetName.innerHTML = HTML;
await keyDown({ key: "Enter" });

expect(sheetName.getAttribute("contenteditable")).toEqual("false");
await nextTick();

expect(sheetName.innerText).toEqual("HELLO");
});
});

test("Can't rename a sheet in readonly mode", async () => {
Expand Down
9 changes: 9 additions & 0 deletions tests/setup/jest.setup.ts
Expand Up @@ -12,6 +12,15 @@ export let OWL_TEMPLATES: Document;
beforeAll(() => {
OWL_TEMPLATES = getParsedOwlTemplateBundle();
setDefaultSheetViewSize(1000);
Object.defineProperty(Element.prototype, "innerText", {
get: function myProperty() {
return this.textContent;
},
set: function myProperty(value) {
this.textContent = value;
this.innerHTML = value;
},
});
});

beforeEach(() => {
Expand Down

0 comments on commit c443de6

Please sign in to comment.