Skip to content

Add support for using EventListener values in a ReactSpec with access to ReactThis #60

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
iand675 opened this issue Feb 22, 2016 · 9 comments
Labels
type: enhancement A new feature or addition.

Comments

@iand675
Copy link

iand675 commented Feb 22, 2016

I am trying to implement a component that listens to the resize event. Essentially it's just a variation on the cookbook example here. Here's the code that I'm trying to write, although it sort of seems to not be possible. The basic issue is that I can create an EventListener, but I don't have a way to get access to the ReactThis inside of it. If I create a lambda that takes a this argument instead of a fully saturated value, then it creates a different event listener when it tries to remove the event listener on componentWillUnmount.

topBar :: ReactClass Unit
topBar = createClass barSpec
  where
    barSpec = (spec unit render)
      { displayName = "top-bar"
      , componentDidMount = \this -> do
          w <- window
          addEventListener resize handler false $ windowToEventTarget w
          return unit
      , componentWillUnmount = \this -> do
          w <- window
          removeEventListener resize (removeEventWriteAccess handler) false $ windowToEventTarget w
          return unit
      }

    handler = eventListener $ \this event -> do
      b <- getCurrentBreakpoint
      case b of
        Small -> writeState this True
        _ -> writeState this False
      print b
@ethul
Copy link
Contributor

ethul commented Feb 22, 2016

Thanks for the report. I see two potential solutions here.

  1. We could perhaps add wrappers for eventListener, addEventListener, and removeEventListener in order to provide the correct this value. However, I will have to dig into the details a bit more on this.
  2. I believe it should be possible to achieve your use-case by storing the event handler (applied with this) in the state of your react component in the componentDidMount. Then in the componentWillUnmount, get the handler from the state and remove it. This is perhaps not ideal, but I think it would work.

@iand675
Copy link
Author

iand675 commented Feb 22, 2016

Storing the handlers in the state works with one caveat– you generally want to be able to write to the state and props, so the tracked effects in the handler includes state :: ReactState (read :: Read, write :: Write). When you remove the event in componentWillUnmount, the effects from the handler are unified with the componentWillUnmount effects which only allows state to be read-only.

I had to write a coercion function to get types to unify since removing the handler doesn't actually trigger write effects:

removeEventWriteAccess :: forall e. EventListener (state :: ReactState ( write :: Write, read :: Read ) | e) -> EventListener (state :: ReactState (read :: Read) | e)
removeEventWriteAccess = unsafeCoerce

@ethul
Copy link
Contributor

ethul commented Feb 23, 2016

Thanks for trying this out. Good point regarding the effects. The unsafeCoerce is not ideal, but perhaps this can suffice until I investigate option (1) a bit more. I think having support for adding/removing of event handlers would be a good addition.

PS - Would unsafeInterleaveEff work?

@iand675
Copy link
Author

iand675 commented Feb 23, 2016

Oh yeah, looks like unsafeInterleaveEff is what I wanted. Thanks for the tip!

@ethul
Copy link
Contributor

ethul commented Feb 23, 2016

Welcome!

@ethul
Copy link
Contributor

ethul commented Apr 9, 2016

@iand675 is this okay to close? I am thinking unsafeInterleaveEff might suffice here. I don't know if we need specific functionality in purescript-react, but I am open to look more into this.

@iand675
Copy link
Author

iand675 commented Apr 9, 2016

I think the one other issue is that you have to put handlers in a Maybe since you can't provide set up a handler in some cases until you get to the componentDidMount callback. It's a bit awkward. Obviously there are workarounds to convince purescript-react to work, but I'm still of the opinion that having some wrappers would be the "correct" solution to this from a user-friendliness perspective.

@ethul
Copy link
Contributor

ethul commented Apr 9, 2016

Good point. I will keep this issue around. I might have some time soon to dig more into this. Thanks!

@ethul ethul added the type: enhancement A new feature or addition. label Apr 9, 2016
@ethul
Copy link
Contributor

ethul commented Mar 4, 2018

As of #129, event handler references can be stored using purescript mechanism; e.g., #124 (comment)

Please reopen if this is still an issue.

@ethul ethul closed this as completed Mar 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A new feature or addition.
Development

No branches or pull requests

2 participants