Skip to content
This repository has been archived by the owner on Mar 1, 2024. It is now read-only.

if dismissOnScroll at false, the click on the close button do not close the banner #16

Closed
F4b1n0u opened this issue May 31, 2016 · 9 comments
Assignees
Labels

Comments

@F4b1n0u
Copy link
Contributor

F4b1n0u commented May 31, 2016

it's the setState() into removeOnScrollListener who trigger the getBanner into the render
witch make disappear it once click on the close button.
but if a props dismissOnScroll at false, listeningScroll is at false as well
and that skips the removeOnScrollListener call
so no render, so the banner do not disappear.

@gabro
Copy link
Member

gabro commented May 31, 2016

I'm not sure I understand.

Can you describe the steps to reproduce this issue and the expected outcome?

@gabro gabro added the bug label May 31, 2016
@F4b1n0u
Copy link
Contributor Author

F4b1n0u commented May 31, 2016

if you put the props dismissOnScroll at false, the close button does not work anymore

@F4b1n0u
Copy link
Contributor Author

F4b1n0u commented May 31, 2016

solved by adding a dummy setState at the end of the onAccept function
but I can't push because of a 403

@gabro
Copy link
Member

gabro commented May 31, 2016

oh I see. Yes, there should be either a dummy setState or a forceUpdate call.

Good catch!

I'll fix it soon

@gabro gabro self-assigned this May 31, 2016
@F4b1n0u
Copy link
Contributor Author

F4b1n0u commented May 31, 2016

I'm in favour of the setState :)
forceUpdate sounds like a hack

@gabro
Copy link
Member

gabro commented May 31, 2016

calling an empty setState is equally an hack. forceUpdate is simply more explicit :)

@F4b1n0u
Copy link
Contributor Author

F4b1n0u commented May 31, 2016

fair enough :)

@F4b1n0u
Copy link
Contributor Author

F4b1n0u commented May 31, 2016

and why not use something like:

  getInitialState: function(){
    return {
      ...
      displayBanner: this.hasAcceptedCookies()
      ...
    };
  },
  onAccept: function() {
    ...
    this.setState({
      displayBanner: true;
    })
  },
  getBanner: function() {
    ...
    if(  this.hasAcceptedCookies() ) {
      return null;
    }
    ...
  },
  render: function() {
    var displayBanner = this.state.displayBanner;
    ....
    return displayBanner ? this.getBanner();
  }

@gabro
Copy link
Member

gabro commented May 31, 2016

@F4b1n0u fixed and released as 0.0.13. Thanks for the report!

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

No branches or pull requests

2 participants