strange toHaveFocus() behaviour #53

artooras · 2018-09-03T07:43:31Z

Sorry for posting the question here, but I have tried spectrum.chat and SO without much success so far...

I'm having trouble understanding how toHaveFocus() works exactly. Here's my setup:

MyComponent.js

import React from 'react'
import styled from 'styled-components'

import TextArea from './TextArea'


const Container = styled.div`
  flex: 1;
  height: 100%;
  padding: ${props => props.theme.size};
`

const Title = styled(TextArea)`
  font-weight: bold;
  font-size: ${props => props.theme.sizeLarger};
  margin-left: ${props => props.theme.sizeSmall};
`

class MyComponent extends React.Component {

  handleTitleChange = e => {
    this.props.onTitleChange(e.target.value)
  }

  handleTitleLostFocus = () => {
    this.props.onChangeComplete()
  }

  render () {
    return (
      <Container>
        <Title 
          value={this.props.item.title || ''}
          onChange={this.handleTitleChange}
          onBlur={this.handleTitleLostFocus}
        />
      </Container>
    )
  }
}

export default MyComponent

MyComponent.test.js

import React from 'react'
import {render, fireEvent, prettyDOM} from 'react-testing-library'

import MyComponent from '../MyComponent'


describe('MyComponent', () => {
  it('handles title changes', () => {
    const title = 'title'
    const handleTitleChangeMock = jest.fn()
    const {getByText} = render(
      <MyComponent
        item={{
          title: title
        }}
        onTitleChange={handleTitleChangeMock}
        onChangeComplete={() => {}}
      />
    )
    const titleInput = getByText(title)
    console.log(prettyDOM(titleInput))
    fireEvent.click(getByText(title))
    expect(getByText(title)).toHaveFocus()
    fireEvent.change(getByText(title), {target: {value: title.slice(0, -1)}})
    expect(handleTitleChangeMock).toHaveBeenCalledTimes(1)
    expect(handleTitleChangeMock).toHaveBeenCalledWith(title.slice(0, -1))
  })
})

When I do this:

const titleInput = getByText(title)
console.log(prettyDOM(titleInput))

The console logs the following:

<textarea
    class="sc-htoDjs dLjZCT sc-bxivhb jdLTBU"
    style="height: 0px;"
  >
    title
  </textarea>

That textarea element is the one I am targeting. But then, when I do this:

fireEvent.click(titleInput)
expect(titleInput).toHaveFocus()

I get this error:

Received:
  <body><textarea style="min-height: 0 !important; max-height: none !important; height: 0px !important; visibility: hidden !important; overflow: hidden !important; position: absolute !important; z-index: -1000 !important; top: 0px !important; right: 0px;" /><div><div class="sc-bZQynM ePHCfO"><textarea class="sc-htoDjs dLjZCT sc-bxivhb jdLTBU" style="height: 0px;">title</textarea></div></div></body>

  at Object.it (src/__tests__/MyComponent.test.js:84:30)
      at new Promise (<anonymous>)
  at Promise.resolve.then.el (node_modules/p-map/index.js:46:16)
      at <anonymous>
  at process._tickCallback (internal/process/next_tick.js:188:7)

I don't quite understand why when I'm trying to assert a textarea element toHaveFocus(), I'm receiving an error that references the entire DOM tree under body...

The text was updated successfully, but these errors were encountered:

gnapse · 2018-09-03T13:37:00Z

Hi @artooras, sorry for the lack of attention in SO.

I'll be able to take a deeper look at this tomorrow. At first sight I'm not sure what it is. It'd be good if you can provide a small reproducible example in codesandbox.io.

Although one final note: You do not need to simulate the click and the focus before fireEvent.change. If all you're trying to do is test that the component properly reacts to attempts to change the texture value, the fireEvent.change is enough.

This of course does not solve the underlying problem, but my point it, you can go forward with the test without those steps. Unless they're there for some reason. Are they?

smacpherson64 · 2018-09-03T20:08:43Z

Hi @artooras, the toHaveFocus matcher checks the element against document.activeElement. By default, and when no element is focused, document.activeElement is set to document.body.

I believe fireEvent.click is not focusing the element in this scenario:
https://codesandbox.io/s/l9j2zzny9l

artooras · 2018-09-15T09:21:22Z

Thanks @smacpherson64. Why doesn't fireEvent.click focus the textarea though? I suppose testing for a click would be more correct than setting focus programmatically via fireEvent.focus as I want to reproduce an actual user action, right?

@gnapse, as for the change event, I'm not sure I need it TBO. Ideally I would like to follow the exact steps a user would, e.g. fireEvent.focus(titleInput) and then fireEvent.keyDown(titleInput, {key: 'a', keyCode: 65, which: 65}). Would this trigger a change?

smacpherson64 · 2018-09-15T14:00:10Z

Hey @artooras,

I did some testing on this and think it is because a manual click in the browser couples a focus event with it, where firing the click event only fires a click event:

Exploration using about:blank

document.body.appendChild(document.createElement('input'))
document.addEventListener('focusin', console.log)
document.addEventListener('click', console.log)
const input = document.querySelector('input')

When clicking on the element manually both the focusin and click event occur.
When using events (input.click()) to click only the 'click' event occurs.

smacpherson64 · 2018-09-15T14:51:58Z

Hahaha, sorry @artooras I didn't fully answer the question. Since the tooling has different behavior than the browser it is necessary in this case to work with the tooling (or change it for the better :-)!). I think you will run into the same with the change event as well. I do not believe, react-testing-library currently binds multiple events together like a browser does.

I think you bring up really good points, the points may have already been discussed but I am not sure. A good avenue for this would be to create a bug report on react-testing-library describing the discrepancies you are experiencing between the browser and react-testing-library. That would bring to light what the correct way to accomplish what you are aiming for when it comes to react-testing-library.

I hope this helps!

gnapse · 2018-09-16T01:59:01Z

@artooras I think the problem is related to the fact that when an element looses focus, the next element to acquire it actually gets it on the next tick, and briefly, during a tick, the focus is placed on the document body. Source: https://stackoverflow.com/a/11299876

Can you please wrap your expectation toHaveFocus inside wait?

// import `wait` from react-testing-library
import { render, fireEvent, cleanup, wait } from 'react-testing-library';

// ...

fireEvent.click(titleInput)
await wait(() => {
  expect(titleInput).toHaveFocus()
});

When you programmatically give it focus directly, instead of with a click, I think it happens immediately. But with a click (or when acquired via pressing the tab key in a web page) it happens in this manner described above. Agreed is weird, because even if it's async, I would expect to pass focus directly from the current active element to the new one, without briefly giving focus to the body. But that's how it is, apparently.

Let us know if this helps.

Update: BTW, this is not a jsdom thing. I actually realized this was the trouble you might be facing, when I encountered it precisely today in a UI I've been working on. It happens in the browser too. That's where I encountered first, not in tests.

smacpherson64 · 2018-09-18T01:41:49Z

Hey @gnapse,

I’ve updated the exploration in codesandbox to use wait but the test is failing: (The wait takes about 5 seconds to fail because of the timeout.)

https://codesandbox.io/s/l9j2zzny9l

Can you check to make sure It matches what you said above or explain further what you mean (if you have a shareable example that would be awesome!). I have not heard of that behavior before!

gnapse · 2018-09-18T02:08:23Z

Ok, I'll dig up a little bit more.

I did not know about this behavior I commented before. I just found out recently when it actually helped me solve a problem unrelated to jest-dom. I thought it could explain this too.

gnapse · 2018-09-18T02:42:18Z

Hmmm, I wonder if dom-testing-library's fireEvent.focus(...) or fireEvent.click(...) merely dispatch the event so that registered event handlers are called, or if it also generates the side effect of giving focus to the element.

A couple of tests I'm doing seem like it's just about triggering event handlers. But if what I suspect is wrong, then I'm not sure what are we (me and @artooras who raised this issue above) doing wrong. The code for fireEvent also does not seem to taking care of this.

I checked our own tests of toHaveFocus, and of course we're giving focus to an element directly by calling element.focus(), because we do not depend on or use dom-testing-library.

@kentcdodds care to chime in?

gnapse · 2018-09-19T23:18:10Z

The best I can come up with is that you'd need to call element.focus() in addition to fireEvent.focus(element), if you not just want to test the event handlers, but also have the actual element receive focus.

At most, it may be conceivable (?) to suggest dom-testing-library to do this ad-hoc action specifically for the fireEvent.focus() call. What do you think @kentcdodds? Though I'd understand if this is not accepted.

kentcdodds · 2018-09-19T23:28:25Z

Huh... Interesting. I think that between this and #59, we're starting to see that what people want is something more like a user object which I've considered before. Whereas fireEvent simply dispatches events on DOM nodes, a user object would fire all events necessary to do an action.

For example, this would focus the input and dispatch input events for each character:

user.type(inputNode, 'hello')

Why don't we open something up in dom-testing-library to discuss this.

gnapse · 2018-09-20T02:28:23Z

Done testing-library/dom-testing-library#107

gnapse mentioned this issue Sep 19, 2018

toHaveStyle ignores hover styles #59

Closed

gnapse mentioned this issue Sep 20, 2018

Expressing user actions at a higher level testing-library/dom-testing-library#107

Closed

gnapse closed this as completed Sep 20, 2018

fraincs mentioned this issue Feb 11, 2020

Issue/167 gsoft-inc/sg-orbit#180

Merged

tbusillo mentioned this issue May 20, 2024

fix(react): make focusOnInitialRender work without setTimeout dequelabs/cauldron#1486

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

strange toHaveFocus() behaviour #53

strange toHaveFocus() behaviour #53

artooras commented Sep 3, 2018

gnapse commented Sep 3, 2018

smacpherson64 commented Sep 3, 2018

artooras commented Sep 15, 2018

smacpherson64 commented Sep 15, 2018

smacpherson64 commented Sep 15, 2018

gnapse commented Sep 16, 2018 •

edited

smacpherson64 commented Sep 18, 2018 •

edited

gnapse commented Sep 18, 2018

gnapse commented Sep 18, 2018 •

edited

gnapse commented Sep 19, 2018

kentcdodds commented Sep 19, 2018

gnapse commented Sep 20, 2018

strange toHaveFocus() behaviour #53

strange toHaveFocus() behaviour #53

Comments

artooras commented Sep 3, 2018

gnapse commented Sep 3, 2018

smacpherson64 commented Sep 3, 2018

artooras commented Sep 15, 2018

smacpherson64 commented Sep 15, 2018

smacpherson64 commented Sep 15, 2018

gnapse commented Sep 16, 2018 • edited

smacpherson64 commented Sep 18, 2018 • edited

gnapse commented Sep 18, 2018

gnapse commented Sep 18, 2018 • edited

gnapse commented Sep 19, 2018

kentcdodds commented Sep 19, 2018

gnapse commented Sep 20, 2018

gnapse commented Sep 16, 2018 •

edited

smacpherson64 commented Sep 18, 2018 •

edited

gnapse commented Sep 18, 2018 •

edited