feat: Place the caret at the start/end (all text elements) or clicked point (contenteditable only) #8529

sainthkh · 2020-09-09T06:27:45Z

Closes Cannot focus element inside content editable element #2717
Closes Allow clicking to set the caret position in ContentEditable components. #5721
Closes Click on table cell inside a contenteditable element does not behave as expected #7721

User facing changelog

Users can choose to place the caret at the start or end of the text elements(<input>, <textarea>)

When a contentdeditable element is clicked, the caret can be placed at the clicked point.

Additional details

Why was this change necessary? => Some users wanted to test their contenteditable is edited correctly but it was impossible.
What is affected by this change? => N/A
Any implementation details to explain? => I used caretPositionFromPoint and caretRangeFromPoint. Because of that, it only works with [contenteditable].

How has the user experience changed?

N/A

PR Tasks

Have tests been added/updated?
Has the original issue been tagged with a release in ZenHub?
Has a PR for user-facing changes been opened in cypress-documentation? => coming soon...
Have API changes been updated in the type definitions?
[N/A] Have new configuration options been added to the cypress.schema.json?

cypress-bot · 2020-09-09T06:27:48Z

Thanks for taking the time to open a PR!

Create a Draft Pull Request if your PR is not ready for review. Mark the PR as Ready for Review when you're ready for a Cypress team member to review the PR.
Become familiar with the Code Review Checklist for guidelines on coding standards and what needs to be done before a PR can be merged.

sainthkh · 2020-09-10T09:36:52Z

Flaky failures.

kuceb · 2020-09-11T18:38:30Z

packages/driver/cypress/integration/commands/actions/click_spec.js

+      // caretPositionFromPoint doesn't work on headless mode on Firefox.
+      if (Cypress.isBrowser('chrome')) {


We should try to fix this not working in headless firefox. This could be really confusing to users whose tests fail in runMode but pass in openMode.

If we were going to ship this we would need to print a warning in headless firefox if the option is supplied.

We also would need to write some tests to cover the behavior in headed firefox

The cause of the problem is caretPositionFromPoint function. It simply returns 0 in headless mode. I fixed this test by adding Cypress.config('isInteractive') === true.

I added error message about skipping this test.

kuceb · 2020-09-11T18:39:17Z

cli/types/cypress.d.ts

@@ -2393,6 +2393,13 @@ declare namespace Cypress {
     * @default false
     */
    cmdKey: boolean
+    /**
+     * Where the caret should be placed when <input>, <textarea>, [contenteditable] elements are clicked.
+     * 'point' option can be only used with [contenteditable].


we should add some logic to throw an error if caretPosition: 'point' is given to a non [contenteditable] element

I added an error message.

kuceb · 2020-09-11T18:42:05Z

packages/driver/src/cy/mouse.js

+        } else if (caretPosition === 'point') {
+          const { x, y } = getMouseCoords(state)
+
+          $selection.moveSelectionToPoint(state('window'), x, y)
+        }


similarly needs onlyIfEmptySelection: true to fulfill the browser default action if selection is changed during the prev event

It seems that it's impossible. To use onlyIfEmptySelection, We need element. But in this case, we don't use element. We just use clientX and clientY.

jennifer-shehane · 2020-10-06T06:50:53Z

What happened to the tests :(

jennifer-shehane · 2020-10-07T07:33:19Z

@sainthkh I'm not sure what's happened to the tests in here - seems specific to this PR.

sainthkh · 2020-10-07T08:17:50Z

I'll check it out tomorrow.

sainthkh · 2020-10-08T08:59:01Z

@jennifer-shehane The bug happened because of a circular reference in files. I'm finding ways to solve that problem.

kuceb

considering this proposed solution does not work in firefox headless, I think there's a better solution here.

Right now we grab the host contenteditable if the subject is inside a contenteditable. Users want the ability to target specific elements inside, not necessarily where the click hits.

So we could change behavior when targeting contenteditables to select the provided subject instead of the host contenteditable. so if the user does cy.get('div[contenteditable] > div > p') we put the cursor at the end of <p> instead of div[contenteditable]. This could use the simple selection.selectNodeContents api to achieve that. Do you think that would meet users' needs @sainthkh? It would be a breaking change

sainthkh · 2020-10-20T06:27:33Z

@bkucera It's yes and no.

It would solve the problems in Cannot focus element inside content editable element #2717 and Click on table cell inside a contenteditable element does not behave as expected #7721 because they don't care the exact position of the caret.
But it doesn't solve Allow clicking to set the caret position in ContentEditable components. #5721 because the user wants to place the caret at the precise position.

I think #5721 isn't an invalid request because there are cases like putting the caret at a precise position is necessary to test WYSIWYG editors. (Example scenario: Click a position in the middle of a sentence. Ctrl + b to turn on bold mode. Type a word. Check if the new word is shown in bold.)

And I'm also afraid that breaking cy.get() is too risky because it's used a lot. Maybe there are a lot of users who are comfortable with the current behavior.

My answer for your suggestion is this:

Add selectRootContenteditable option to get. Default is true. If someone wants to select something with elements inside [contenteditable] set it to false. It would solve #2717 and #7721.

The problem is #5721. For some developers, current 'point' option is definitely better than nothing. But it's a half-done feature because it doesn't work on Firefox headless (in exact words, when Firefox GUI cannot be executed).

I believe there are 3 options for now:

Release it. Document and throw error when used with Firefox.
Discard it. And revisit this when the bug in caretPositionFromPoint is fixed.
Release it as a plugin. And maybe merge this to this repo when the bug is fixed.

What do you think?

kuceb · 2020-10-20T19:32:35Z

@sainthkh this would not affect cy.get, only what element cy.type and cy.click target, so it wouldn't be that big of a breaking change. cy.type already has logic around contenteditable subjects

as for #5721 I dont believe this PR fixes the issue. That issue is asking for an option like { cursorPosition: 3 }, not the clicked point. I don't believe using the clicked point is not precise or predictable enough for testing. For example a simple padding change could move the click from character 3 to character 4, since we target the center of the element by default.

sainthkh · 2020-10-20T21:33:06Z

@bkucera I agree with you. I'll close this PR and open a new one.

sainthkh force-pushed the issue-7721 branch 2 times, most recently from 780a35c to 312ddda Compare September 10, 2020 09:14

sainthkh marked this pull request as ready for review September 10, 2020 09:36

flotwig requested review from kuceb and chrisbreiding September 11, 2020 16:28

kuceb suggested changes Sep 11, 2020

View reviewed changes

sainthkh force-pushed the issue-7721 branch from 312ddda to b35736c Compare September 14, 2020 08:10

jennifer-shehane requested a review from kuceb October 6, 2020 06:02

jennifer-shehane added the internal-priority label Oct 6, 2020

sync-by-unito bot assigned kuceb Oct 6, 2020

sainthkh added 7 commits October 8, 2020 18:12

Add caretPosition option to click.

6eb8917

Add type definition.

220a347

Fix firefox error

9500594

Skip Firefox headless mode.

103cab4

Remove Firefox support.

ad858d8

Add comment about the issue.

7759e91

Feedback.

3dfd2ed

sainthkh force-pushed the issue-7721 branch from 7d272fb to 3dfd2ed Compare October 8, 2020 09:18

kuceb suggested changes Oct 19, 2020

View reviewed changes

sainthkh closed this Oct 20, 2020

sainthkh mentioned this pull request Nov 3, 2020

feat: Allow type() and click() to select an element inside contenteditable. #9066

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Place the caret at the start/end (all text elements) or clicked point (contenteditable only) #8529

feat: Place the caret at the start/end (all text elements) or clicked point (contenteditable only) #8529

sainthkh commented Sep 9, 2020 •

edited by sync-by-unito bot

Loading

cypress-bot bot commented Sep 9, 2020

sainthkh commented Sep 10, 2020

kuceb Sep 11, 2020

sainthkh Sep 14, 2020

kuceb Sep 11, 2020

sainthkh Sep 14, 2020

kuceb Sep 11, 2020

sainthkh Sep 14, 2020

jennifer-shehane commented Oct 6, 2020

jennifer-shehane commented Oct 7, 2020

sainthkh commented Oct 7, 2020

sainthkh commented Oct 8, 2020

kuceb left a comment

sainthkh commented Oct 20, 2020

kuceb commented Oct 20, 2020 •

edited

Loading

sainthkh commented Oct 20, 2020

		// caretPositionFromPoint doesn't work on headless mode on Firefox.
		if (Cypress.isBrowser('chrome')) {

feat: Place the caret at the start/end (all text elements) or clicked point (contenteditable only) #8529

feat: Place the caret at the start/end (all text elements) or clicked point (contenteditable only) #8529

Conversation

sainthkh commented Sep 9, 2020 • edited by sync-by-unito bot Loading

User facing changelog

Additional details

How has the user experience changed?

PR Tasks

cypress-bot bot commented Sep 9, 2020

sainthkh commented Sep 10, 2020

kuceb Sep 11, 2020

Choose a reason for hiding this comment

sainthkh Sep 14, 2020

Choose a reason for hiding this comment

kuceb Sep 11, 2020

Choose a reason for hiding this comment

sainthkh Sep 14, 2020

Choose a reason for hiding this comment

kuceb Sep 11, 2020

Choose a reason for hiding this comment

sainthkh Sep 14, 2020

Choose a reason for hiding this comment

jennifer-shehane commented Oct 6, 2020

jennifer-shehane commented Oct 7, 2020

sainthkh commented Oct 7, 2020

sainthkh commented Oct 8, 2020

kuceb left a comment

Choose a reason for hiding this comment

sainthkh commented Oct 20, 2020

kuceb commented Oct 20, 2020 • edited Loading

sainthkh commented Oct 20, 2020

sainthkh commented Sep 9, 2020 •

edited by sync-by-unito bot

Loading

kuceb commented Oct 20, 2020 •

edited

Loading