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

[IMP] LinkDisplay: update grid selection before opening the editor #3809

Closed
wants to merge 1 commit into from

Conversation

rrahir
Copy link
Collaborator

@rrahir rrahir commented Mar 9, 2024

Currently, a user can open the link editor of a cell while hovering it but not selectd. This behaviour is counterintuitive as once the link editor is opened, we will close it if we change the selection.

This revision ensures that the link editor target and the selection are synchronized upon the initialization of the the former.

Task: 3793859

Description:

description of this task, what is implemented and why it is implemented that way.

Task: : TASK_ID

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 9, 2024

onMounted(() => this.urlInput.el?.focus());
onMounted(() => {
const { col, row } = this.props.cellPosition;
this.env.model.selection.selectCell(col, row);
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe select the cell in link_display.ts before opening the component to avoid modifying the state here, which would trigger a double render?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agreed :) change made and update the commit message

@rrahir rrahir force-pushed the master-sync-link-editor-and-selection-rar branch from a5aaf38 to 353da93 Compare March 22, 2024 09:58
@rrahir rrahir changed the title [IMP] LinkEditor: Editor target is synced with the selection [IMP] LinkDisplay: update grid selection before opening the editor Mar 22, 2024
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.

👍

tests/link/link_editor_component.test.ts Outdated Show resolved Hide resolved
Currently, a user can open the link editor of a cell while hovering it
but not selectd. This behaviour is counterintuitive as once the link
editor is opened, we will close it if we change the selection.

This revision ensures that the link editor target and the selection are
synchronized upon the initialization of the the former.

Task: 3793859
@LucasLefevre LucasLefevre force-pushed the master-sync-link-editor-and-selection-rar branch from 353da93 to c4cb2fd Compare March 22, 2024 13:35
@LucasLefevre
Copy link
Collaborator

robodoo r+

robodoo pushed a commit that referenced this pull request Mar 22, 2024
Currently, a user can open the link editor of a cell while hovering it
but not selectd. This behaviour is counterintuitive as once the link
editor is opened, we will close it if we change the selection.

This revision ensures that the link editor target and the selection are
synchronized upon the initialization of the the former.

closes #3809

Task: 3793859
Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com>
@robodoo robodoo closed this Mar 22, 2024
@robodoo robodoo added the 17.3 label Mar 22, 2024
@fw-bot fw-bot deleted the master-sync-link-editor-and-selection-rar branch April 5, 2024 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants