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

Notification not displayed after route change #42

Open
j-gurda opened this issue Aug 1, 2017 · 2 comments
Open

Notification not displayed after route change #42

j-gurda opened this issue Aug 1, 2017 · 2 comments

Comments

@j-gurda
Copy link

j-gurda commented Aug 1, 2017

I'm currently investigating issue related to notification not being displayed after route change. Following scenario describes the case:

  1. I have separate route which presents form for adding new entity
  2. When I click "Save" button AJAX request is dispatched and on success of that request I dispatch new notification and immediately change route using history.push('/some-url')

Following pseudo-code presents that:

function onSuccess(data){
   dispatch(Notifications.success( .... ));
   history.push('/some-url');
}
  1. After route is changed notification is not displayed however notification is still inside Redux store. Also no RNS_HIDE_NOTIFICATION is fired.
  2. When I invoke an action which displays another notification but does not change the route "old" notifications are displayed too what looks like queuing up of notifications.. All notifications disappear after expected time.

Here's list of dependencies:

    "axios": "^0.16.2",
    "common-tags": "^1.4.0",
    "immutability-helper": "^2.3.0",
    "jwt-decode": "^2.2.0",
    "prop-types": "^15.5.10",
    "query-string": "^5.0.0",
    "react": "^15.5.4",
    "react-autosize-textarea": "^0.4.8",
    "react-bootstrap": "^0.31.0",
    "react-bootstrap-typeahead": "^2.0.0-alpha.1",
    "react-collapsible": "^1.5.0",
    "react-cookies": "0.0.1",
    "react-dom": "^15.5.4",
    "react-html-id": "^0.1.1",
    "react-notification-system": "^0.2.14",
    "react-notification-system-redux": "^1.1.4",
    "react-numeric-input": "^2.0.7",
    "react-redux": "^5.0.5",
    "react-router-bootstrap": "^0.24.2",
    "react-router-dom": "^4.1.1",
    "redux": "^3.6.0",
    "redux-form": "^6.7.0",
    "redux-thunk": "^2.2.0"

I'm relatively new to React and Redux and I'm trying to build minimal app which reproduces that issue but maybe you could figure out faster what's going on.

@j-gurda
Copy link
Author

j-gurda commented Aug 22, 2017

I found cause of an issue. I used Route component higher in component hierarchy than Notifications component. That caused need of mounting Notifications component every time route was changed.
Here is sample pseudo code:

<Route exact 
    path="/" 
    component={DashboardPage}>
    <Notifications
           notifications={notifications}/>
</Route>

In source code of Notifications you refresh messages only on componentWillReceiveProps so when Notifications is freshly mounted componentWillReceiveProps is not called.

There are two possible solutions for that:

  1. Use Notifications above Route component so changing route does not cause remounting of it:
<Notifications
           notifications={notifications}/>
<Route exact 
    path="/" 
    component={DashboardPage}/>   
<Route exact 
    path="/some_other_url" 
    component={OtherPage}/>  
  1. Patch Notifications component to update notifications also on componentDid(Will)Mount.

Thank you for great piece of software!

@gor181
Copy link
Owner

gor181 commented Sep 28, 2017

Thank you for reporting the detailed use case, I'm really glad that you are finding the lib as useful.
I agree that we should add behavior of updating the notifications on will/didmount as you suggested.

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

2 participants