Skip to content

focusIn, focusOut, and resize events #179

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

Closed
sarenji opened this issue Dec 11, 2018 · 8 comments · Fixed by #182 or #990
Closed

focusIn, focusOut, and resize events #179

sarenji opened this issue Dec 11, 2018 · 8 comments · Fixed by #182 or #990
Labels

Comments

@sarenji
Copy link
Contributor

sarenji commented Dec 11, 2018

Describe the feature you'd like:

fireEvent.focusIn and fireEvent.focusOut seem to be missing. I didn't know if there was a reasoning for it, so I wanted to double-check with y'all. Here's the documentation on focusin: https://developer.mozilla.org/en-US/docs/Web/Events/focusin

Additionally, fireEvent.resize is missing. This one is a bit weirder, though, so rather than suggest anything, I'm asking if it belongs in here.

Suggested implementation:

Note that focusIn and focusOut bubble, unlike focus and blur.

// https://github.com/kentcdodds/dom-testing-library/blob/master/src/events.js
const eventMap = {
  // ...
  focusIn: {
    EventType: 'FocusEvent',
    defaultInit: {bubbles: true, cancelable: false},
  },
  focusOut: {
    EventType: 'FocusEvent',
    defaultInit: {bubbles: true, cancelable: false},
  },
  // ...
}

In the case of resize, it's a little weird. You want to call it on a Window. The magic function that gets assigned to all these events tries to find the window via const window = node.ownerDocument.defaultView, which does an angry tableflip if you pass it a window in the first place. So some ceremony can be skipped:

fireEvent.resize = (window) => {
  window.dispatchEvent(new Event('resize'));
};

But seeing as it's a little different than the rest and a one-liner I'm not bothered if y'all think adding resize is a bad idea.

I'm happy to make a PR for either.

Describe alternatives you've considered:

We can just use the low-level version (i.e. just fireEvent) in all cases, but it'd be nice to have the convenience functions for a consistent API.

Teachability, Documentation, Adoption, Migration Strategy:

Documentation is sparse; I think the library just links to src/events.js.

Thoughts? 😄

@kentcdodds
Copy link
Member

Thanks! If it's standard, then it should probably be included. Yes, please do make a PR! Thank you.

@kentcdodds
Copy link
Member

🎉 This issue has been resolved in version 3.16.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@DylanPiercey
Copy link
Contributor

Looks like fireEvent.resize was missing from #182, are PR's still welcome for that @kentcdodds?

@kentcdodds
Copy link
Member

Resize events are unique because they can only be fired on window right? That being the case, I'm not sure how to proceed and I'd like to hear some discussion around it. Could you open a new issue about it please?

@gnapse
Copy link
Member

gnapse commented Nov 3, 2020

FWIW, I managed to fire a resize event in a test like this:

fireEvent(window, new Event('resize'))

Maybe fireEvent should merely check that the event target is always window, and maybe we can add this:

fireEvent.resize(event)

having the first argument be the event and not supporting passing the target element.

@eps1lon
Copy link
Member

eps1lon commented Nov 3, 2020

I had a similar issue with keyDown which can only be fired at the active element. However, I'm not a fan of hiding this i.e. fireEvent.keyDown(event) because it makes for less readable tests. That restriction is not well known so hiding it, doesn't really solve a problem.

resize seems similar to me. We might want to start warning (I'd be careful with throwing because some environment might not implement the DOM strictly) if resize is dispatched at anything but a Window.

Another problem with fireEvent.resize(event): How do you know which Window is targeted? It's ambigious with frames but might even be impossible with JSDOM.

@gnapse
Copy link
Member

gnapse commented Nov 3, 2020

Yup, makes sense. Then maybe leaving the same interface and warning about the target not being a window. That's fine with me.

@eps1lon
Copy link
Member

eps1lon commented Jul 1, 2021

Introducing resize event in #990

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants