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

[Editor] Simplify the way to create an editor on click #15184

Merged
merged 1 commit into from
Jul 19, 2022

Conversation

calixteman
Copy link
Contributor

Previously, we had to set the #allowClick property by hand which was
a bit painful because it's easy to overlook one case or an other.
So with this patch a new editor (for now FreeText one only because the
Ink one is a bit different) is created on the first click if none is selected
on mousedown, else the first click will just commit the data and then the
second will creater a new editor.

@calixteman
Copy link
Contributor Author

And it should help to fix an issue with integration test in #15110.

return;
}

this.#createAndAddNewEditor(event);
}

/**
* Mouseclick callback.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This looks like too much copy-and-paste from the method above, since I assume that you actually want:

Suggested change
* Mouseclick callback.
* Mousedown callback.

/**
* Mouseclick callback.
* @param {MouseEvent} event
* @returns {undefined}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's not add any more of these "returns undefined" JSDoc-comments, since you're removing the existing ones in another PR.

Previously, we had to set the #allowClick property by hand which was
a bit painful because it's easy to overlook one case or an other.
So with this patch a new editor (for now FreeText one only because the
Ink one is a bit different) is created on the first click if none is selected
on mousedown, else the first click will just commit the data and then the
second will creater a new editor.
@calixteman
Copy link
Contributor Author

/botio integrationtest

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

Command cmd_integrationtest from @calixteman received. Current queue size: 0

Live output at: http://54.193.163.58:8877/82cc8f694c4c1c5/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

Command cmd_integrationtest from @calixteman received. Current queue size: 0

Live output at: http://54.241.84.105:8877/d4e1bb55b131ac8/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/d4e1bb55b131ac8/output.txt

Total script time: 4.50 mins

  • Integration Tests: Passed

@pdfjsbot
Copy link

From: Bot.io (Windows)


Failed

Full output at http://54.193.163.58:8877/82cc8f694c4c1c5/output.txt

Total script time: 9.89 mins

  • Integration Tests: FAILED

@calixteman calixteman merged commit 642676a into mozilla:master Jul 19, 2022
@calixteman calixteman deleted the rm_allowclick branch July 19, 2022 15:56
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.

3 participants