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: add implementation for contenteditable input events #235

Conversation

benedictfischer09
Copy link

Firing a change event on an element that is contenteditable enabled throws an error with the following message: The given element does not have a value setter

What:
Changed the fireEvent.change api to transparently check to see if the element is contenteditable and if so, set the element's textContent based on the event's target value.

Why:
Input events are supported on non form elements if the element has contenteditable enabled see https://developer.mozilla.org/en-US/docs/Web/Events/input for more information.

How:
There are no new apis or changes to the interface of existing apis. The existing api handles the contenteditable case without changes to how it handles changes to form elements.

Checklist:

  • Documentation added to the docs site N/A
  • Typescript definitions updated N/A
  • Tests
  • Ready to be merged

One thing I'm not confident about is whether delegating an input event's target value to element's textContent setter is a good idea. In seems to make sense in many situations but probably not all

@alexkrolick
Copy link
Collaborator

This seems like a limitation that should be addressed in JSDOM if possible, but I'm not seeing any plans to support contenteditable yet: jsdom/jsdom#1670

@benedictfischer09
Copy link
Author

Gotcha, are you thinking that if an input event is dispatched then JSDOM should change that element's content based on the event? From what I can tell that is not the case for browsers; dispatching an input event (even on a form input) has no impact on the value property of the element from my tests. So I'd assume JSDOM project probably wouldn't accept that change.

I might be totally misunderstanding the point of setNativeValue but I guess I saw it as a way to make the DOM consistent with what the sythentic event says happened?

@benedictfischer09
Copy link
Author

@alexkrolick based on my previous message do you still think this change doesn't really belong in dom-testing-library project?

@alexkrolick
Copy link
Collaborator

alexkrolick commented Apr 8, 2019

Yes, I think that if polyfills for events in JSDOM should be moved to a separate library or added to JSDOM itself

Especially something as complex as contenteditable

@kentcdodds
Copy link
Member

Thank you for making the effort @benedictfischer09

@smeijer
Copy link
Member

smeijer commented Feb 10, 2020

@benedictfischer09, did you find a workable solution to test forms with a contentEditable?

I'm using a contentEditable with formik, so I depend on the onChange call.

@citypaul
Copy link

+1 for re-considering this functionality.

Unfortunately, at the moment this feature has not been added to JSDOM, but there are people building features in React that require testing ContentEditable, and currently this is not possible with RTL as things currently stand, so far as I can tell.

I'm unable to trigger the onChange, here which means for this feature we're going to have to test the component using Testcafe instead of using RTL, which I would much prefer.

Seeing as right now there is a limitation that prevents testing content editable components, would you be open to re-considering this idea?

@kentcdodds
Copy link
Member

Sorry, but what @alexkrolick said originally still applies. You could definitely do what this PR does to test it if you really want to.

@pstephenwille
Copy link

pstephenwille commented Apr 27, 2020

One thing that worked for me was to use the 'blur' event to update the field. Testing-Library fires the blur event and my input is changed.

            fireEvent.blur(firstNameField, {target: {textContent: 'changed-first-name'}});

norisalsaadie added a commit to libero/reviewer-client that referenced this pull request Jul 8, 2020
Based on this stack overflow (https://stackoverflow.com/questions/60495903/testing-react-contenteditable-with-react-testing-library+&cd=1&hl=en&ct=clnk&gl=uk) which points to two discussions testing-library/dom-testing-library#235 and jsdom/jsdom#1670

It’s not possible to simulate events on a contenteditable div within a js-dom context. This is a limit of js-dom itself. This is a known issue since 2016.

To save the test, the following was reviewed - modifying the test to click the button but the value of the coverLetter would still be null as it requires that DOM event -> dispatchTranscation -> props.onChange to modify the state of coverLetter.
@dominique-mueller
Copy link

Ran into the same issues, in my case when trying to connect a "contenteditable" span to Formik in a React project.

First of all, I had to listen to the input event instead of the change event to successfully capture the value. For instance:

const handleInput = (event) => {
  helper.setValue(event.target.textContent)
}

<span contentEditable onInput={handleInput} />

Then, within the corresponding unit test, I wanted to use React Testing Library's fireEvent.input function, but it doesn't do anything. As a workaround, I manually set the contenteditable text content and manually dispatch an input event. For instance:

contenteditableElement.textContent = 'my value'
contenteditableElement.dispatchEvent(
  new Event('input', {
    bubbles: true,
    cancelable: true,
  }),
)

Funnily enough, using fireEvent.focus() and fireEvent.blur() work just fine.

@danawoodman
Copy link

For the curious, you can also use blur to set innerHTML like so:

  fireEvent.blur(editor, {
    target: { innerHTML: "<p>hello world</p>" },
  });

@nehalahmed929
Copy link

One thing that worked for me was to use the 'blur' event to update the field. Testing-Library fires the blur event and my input is changed.

            fireEvent.blur(firstNameField, {target: {textContent: 'changed-first-name'}});

I also got it working using

            fireEvent.input(firstNameField, {target: {textContent: 'changed-first-name'}});

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.

9 participants