Skip to content
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

[FIX] BottomBarSheet: renaming a sheet with styled content #3351

Closed

Conversation

dhrp-odoo
Copy link
Contributor

@dhrp-odoo dhrp-odoo commented Dec 14, 2023

Description:

Modified the span element to be contenteditable in plain-text only, resolving the traceback issue during sheet renaming.

Task: : 3621086

review checklist

  • feature is organized in plugin, or UI components
  • support of duplicate sheet (deep copy)
  • in model/core: ranges are Range object, and can be adapted (adaptRanges)
  • in model/UI: ranges are strings (to show the user)
  • undo-able commands (uses this.history.update)
  • multiuser-able commands (has inverse commands and transformations where needed)
  • new/updated/removed commands are documented
  • exportable in excel
  • translations (_t("qmsdf %s", abc))
  • unit tested
  • clean commented code
  • track breaking changes
  • doc is rebuild (npm run doc)
  • status is correct in Odoo

@robodoo
Copy link
Collaborator

robodoo commented Dec 14, 2023

@@ -185,7 +185,18 @@ export class BottomBarSheet extends Component<Props, SpreadsheetChildEnv> {
}

private getInputContent(): string | undefined | null {
return this.sheetNameRef.el?.textContent;
const element = this.sheetNameRef.el;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can use contenteditable="plaintext-only" and it would avoid the issue right ?

diff --git a/src/components/bottom_bar_sheet/bottom_bar_sheet.xml b/src/components/bottom_bar_sheet/bottom_bar_sheet.xml
index 98de053af..b79f68091 100644
--- a/src/components/bottom_bar_sheet/bottom_bar_sheet.xml
+++ b/src/components/bottom_bar_sheet/bottom_bar_sheet.xml
@@ -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.toString()"
+          t-att-contenteditable="state.isEditing ? 'plaintext-only': '0'"
         />
         <span class="o-sheet-icon ms-1" t-on-click.stop="(ev) => this.onIconClick(ev)">
           <t t-call="o-spreadsheet-Icon.TRIANGLE_DOWN"/>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absolutely, @LucasLefevre, Updated the code. Thank you for the input!

@dhrp-odoo dhrp-odoo force-pushed the saas-16.2-fix-traceback-rename-sheet-dhrp branch from bd9ab3c to 2be636c Compare January 8, 2024 06:43
Modified the span element to be contenteditable in plain-text only,
resolving the traceback issue during sheet renaming.

Task:: 3621086
@dhrp-odoo dhrp-odoo force-pushed the saas-16.2-fix-traceback-rename-sheet-dhrp branch from 2be636c to c3a5147 Compare January 11, 2024 05:55
Copy link
Collaborator

@LucasLefevre LucasLefevre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

robodoo r+
Thanks :)

robodoo pushed a commit that referenced this pull request Jan 26, 2024
Modified the span element to be contenteditable in plain-text only,
resolving the traceback issue during sheet renaming.

Task:: 3621086

closes #3351

Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com>
@robodoo robodoo closed this Jan 26, 2024
@fw-bot fw-bot deleted the saas-16.2-fix-traceback-rename-sheet-dhrp branch February 9, 2024 13:46
rrahir added a commit that referenced this pull request Mar 25, 2024
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
robodoo pushed a commit that referenced this pull request Mar 25, 2024
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

closes #3894

Task: 3754944
Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com>
rrahir added a commit that referenced this pull request Mar 25, 2024
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
X-original-commit: a11b5dc
robodoo pushed a commit that referenced this pull request Mar 25, 2024
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

closes #3915

Task: 3754944
X-original-commit: a11b5dc
Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com>
Signed-off-by: Rémi Rahir (rar) <rar@odoo.com>
rrahir added a commit that referenced this pull request Mar 25, 2024
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
X-original-commit: 91d7a19
robodoo pushed a commit that referenced this pull request Mar 25, 2024
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

closes #3919

Task: 3754944
X-original-commit: 91d7a19
Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com>
Signed-off-by: Rémi Rahir (rar) <rar@odoo.com>
fw-bot pushed a commit that referenced this pull request Mar 25, 2024
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
X-original-commit: ed7d3fa
fw-bot pushed a commit that referenced this pull request Mar 25, 2024
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
X-original-commit: ed7d3fa
fw-bot pushed a commit that referenced this pull request Mar 25, 2024
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
X-original-commit: ed7d3fa
robodoo pushed a commit that referenced this pull request Mar 25, 2024
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

closes #3922

Task: 3754944
X-original-commit: ed7d3fa
Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com>
Signed-off-by: Rémi Rahir (rar) <rar@odoo.com>
robodoo pushed a commit that referenced this pull request Mar 25, 2024
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

closes #3921

Task: 3754944
X-original-commit: ed7d3fa
Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com>
Signed-off-by: Rémi Rahir (rar) <rar@odoo.com>
robodoo pushed a commit that referenced this pull request Mar 25, 2024
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

closes #3920

Task: 3754944
X-original-commit: ed7d3fa
Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com>
Signed-off-by: Rémi Rahir (rar) <rar@odoo.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants