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] Composer: pressing enter in the link editor #3931

Closed
wants to merge 1 commit into from

Conversation

dhrp-odoo
Copy link
Contributor

Description:

Previously, pressing Enter in the link editor would inadvertently open the grid composer in edit mode, which was not the expected behavior. This PR addresses the issue by adding a check in the onInput method of the composer component.

Task: : 3796114

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 Mar 26, 2024

@@ -364,6 +364,9 @@ export class Composer extends Component<ComposerProps, SpreadsheetChildEnv> {
const el = this.composerRef.el! as HTMLInputElement;
content = el.childNodes.length ? el.textContent! : "";
}
if (!content && this.props.focus === "inactive") {
Copy link
Contributor

Choose a reason for hiding this comment

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

This probably works, but I think we should focus on the underlying cause of the issue: why the hell does the composer receive an input event when pressing enter on the link editor ?

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 it's kind of the same beahaviour with the click event. When you mousedown and and mouseup on differnet targets, the click event is
Worth looking into it at least

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After digging into it, I noticed that when we press enter in the link editor, it triggers the input event in the composer. According to the documentation, this doesn't quite match how the click event behaves.

I tried adding an onInput listener higher up in the hierarchy, like a spreadsheet component, and ran into the same problem. EventPhase tells us that it is happening in the target phase, not during bubbling or capturing.

I am unsure if this is the default behavior or not. Using PreventDefault stops it, but it's likely to cause other problems.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd argue taht preventDefault is exactly what you are looking for. You want to stop the browser chain of events since it makes no sense to keep the "keyboard input event" behaviour once we closed (and destroyed) the input .

Previously, pressing Enter in the link editor would inadvertently open
the grid composer in edit mode, which was not the expected behavior.
This commit addresses the issue by adding a check in the onInput method
of the composer component.

TaskID: 3796114
@dhrp-odoo dhrp-odoo force-pushed the 16.0-fix-link-editor-enter-dhrp branch from 23ea594 to aba4a37 Compare April 2, 2024 09:29
@rrahir
Copy link
Collaborator

rrahir commented Apr 5, 2024

@robodoo r+

robodoo pushed a commit that referenced this pull request Apr 5, 2024
Previously, pressing Enter in the link editor would inadvertently open
the grid composer in edit mode, which was not the expected behavior.
This commit addresses the issue by adding a check in the onInput method
of the composer component.

closes #3931

Taskid: 3796114
Signed-off-by: Rémi Rahir (rar) <rar@odoo.com>
@robodoo robodoo closed this Apr 5, 2024
@fw-bot
Copy link
Collaborator

fw-bot commented Apr 9, 2024

@dhrp-odoo @rrahir this pull request has forward-port PRs awaiting action (not merged or closed):

1 similar comment
@fw-bot
Copy link
Collaborator

fw-bot commented Apr 10, 2024

@dhrp-odoo @rrahir this pull request has forward-port PRs awaiting action (not merged or closed):

@fw-bot
Copy link
Collaborator

fw-bot commented Apr 11, 2024

@dhrp-odoo @rrahir this pull request has forward-port PRs awaiting action (not merged or closed):

1 similar comment
@fw-bot
Copy link
Collaborator

fw-bot commented Apr 12, 2024

@dhrp-odoo @rrahir this pull request has forward-port PRs awaiting action (not merged or closed):

@fw-bot fw-bot deleted the 16.0-fix-link-editor-enter-dhrp branch April 19, 2024 16:47
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

5 participants