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

[Core/Toast] Toaster.create() returns null #1688

Closed
casoetan opened this issue Oct 7, 2017 · 10 comments
Closed

[Core/Toast] Toaster.create() returns null #1688

casoetan opened this issue Oct 7, 2017 · 10 comments

Comments

@casoetan
Copy link

casoetan commented Oct 7, 2017

Bug report

  • Package version(s): @blueprintjs/core": "^1.30.0"
  • Browser and OS versions: Chrome Canary | MacOS 10.12.6

Steps to reproduce

  1. Create a toaster
  2. Try to show it
  3. Fails as toaster is null

Actual behavior

Toast does not show

Expected behavior

Toast to show

@llorca
Copy link
Contributor

llorca commented Oct 7, 2017

Please provide code examples that repros and exact error messages

@casoetan
Copy link
Author

casoetan commented Oct 8, 2017

@llorca apologies for the delay.

The defined method

import { Position, Toaster } from '@blueprintjs/core'

export const appNotification = dismissAction => Toaster.create({
  className: 'app-toaster',
  position: Position.TOP,
  onDismiss: () => dismissAction()
})

The component

import React, { Component } from 'react'
import PropTypes from 'prop-types'

import { bindActionCreators } from 'redux'
import { connect } from 'react-redux'

import { appNotification } from './utils/notification'

import Header from './components/Header'
import Footer from './components/Footer'

import { clearNotification } from './actions/app'

class Layout extends Component {
  constructor(props) {
    super(props)
    this.state = {
      toaster: null
    }
  }

  componentDidMount() {
    this.setState = {
      toaster: appNotification(this.props.actions.clearNotification)
    }
  }

  componentWillReceiveProps(nextProps) {
    const { toaster } = this.state
    const { app: { notification } } = this.props
    const { app: { notification: nextNotification } } = nextProps
    if (notification !== nextNotification) {
      toaster && toaster.show(nextNotification)
    }
  }

  render() {
    const { app } = this.props
    return (
      <div>
        <Header app={app} />
        <div>
          {this.props.children}
        </div>
        <Footer />
      </div>
    )
  }
}

const mapStateToProps = state => ({
  app: state.app
})

const mapDispatchToProps = dispatch => ({
  actions: bindActionCreators({ clearNotification }, dispatch)
})

Layout.propTypes = {
  children: PropTypes.any.isRequired,
  app: PropTypes.object.isRequired,
  actions: PropTypes.shape({
    clearNotification: PropTypes.func
  })
}

export default connect(mapStateToProps, mapDispatchToProps)(Layout)

appNotification is always null, o calling .show(message) on it breaks ...

cannot call method show on null

@AlexLandau
Copy link
Contributor

this.setState = { should be this.setState({, no?

@cmslewis
Copy link
Contributor

cmslewis commented Oct 9, 2017

^ @casoetan - yes, try this and let us know if it fixes your issue.

@cmslewis cmslewis changed the title Toaster.create() returns null [Core/Toast] Toaster.create() returns null Oct 9, 2017
@giladgray
Copy link
Contributor

@casoetan there are a number of bugs in your code, including the setState assignment mentioned above. The most glaring is, of course, that you're not using the Toaster API correctly:
Toaster.create() returns a toaster instance with a show method. It is this show method which accepts onDismiss, so you're passing that handler in the wrong place.

The expected usage is something like this:

  1. Create a Toaster instance using Toaster.create() (typically one instance per position).
    • I like defining this instance in a separate module like common/toaster.ts so any component can simply import it and profit 💸
    export const TopToaster = Toaster.create({ position: TOP, ... })
  2. Call instance.show({ ...props, onDismiss }) to show a toast in that Toaster.
    import { TopToaster } from "../path/to/common/toaster";
    
    // later, in response to user event:
    TopToaster.show({ ...props, onDismiss: this.handleDismiss })

Hope this helps! Please refactor your code and let us know if you can get it working.

@giladgray
Copy link
Contributor

giladgray commented Oct 9, 2017

Oh it's also worth noting that calling Toaster.create inside a component lifecycle method can actually return null (and it definitely returns null in React 16): https://reactjs.org/blog/2017/09/26/react-v16.0.html#breaking-changes (2nd bullet)

Edit: All this to support my proposal of defining a Toaster at initialization time (i.e., import) instead of at render() or mount time.

@casoetan
Copy link
Author

@AlexLandau the set state was obviously from scratching my head for over 2 days trying to get what worked on React 15 to work on React 16.

@giladgray you are quite right. In React 16, this would always return null. #facebook/react#10309 (comment).

The Toaster does not work with SSR as it depends on the dom, reason I'm instantiating in ComponentDidMount.

1 similar comment
@casoetan
Copy link
Author

@AlexLandau the set state was obviously from scratching my head for over 2 days trying to get what worked on React 15 to work on React 16.

@giladgray you are quite right. In React 16, this would always return null. #facebook/react#10309 (comment).

The Toaster does not work with SSR as it depends on the dom, reason I'm instantiating in ComponentDidMount.

@casoetan
Copy link
Author

Solved with #1205

export const Toast = (typeof window !== 'undefined')
   ? Toaster.create({
     className: 'my-toaster',
     position: Position.BOTTOM_RIGHT
   })
  : null

@jrmyio
Copy link

jrmyio commented Nov 15, 2017

I am using a DIC for the toaster, so it calls it whenever its used for the first time.

In this case its also returning null.

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

No branches or pull requests

6 participants