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

emit('popState')... can't get it do anything #621

Closed
timsim00 opened this issue Jan 30, 2018 · 7 comments
Closed

emit('popState')... can't get it do anything #621

timsim00 opened this issue Jan 30, 2018 · 7 comments

Comments

@timsim00
Copy link

Expected behavior

Could be my misunderstanding or using the wrong pattern; seems like emit('popState') should call window.history.back()

Actual behavior

No observable behavior.

Steps to reproduce behavior

choo v.6.6.0

    this.emitter.prependListener(this._events.POPSTATE, function () {
      self.emitter.emit(self._events.NAVIGATE)
    })
  function onClick (e) {
    emit('popState')
    //window.history.back() //has desired affect.
  }
        <button onclick=${onClick}>
          Back
        </button>
@bates64
Copy link
Contributor

bates64 commented Jan 30, 2018

I may be wrong, but I believe popState is for in-app navigation only - eg. to reverse a pushState.

@yoshuawuyts
Copy link
Member

yoshuawuyts commented Jan 30, 2018

created a repro in https://github.com/yoshuawuyts/repro-choo-621, this behavior is confirmed.

In our docs we're saying emit('popState') is indeed to navigate backwards, which is incorrect - it's an event to listen to when we navigate backwards.

There's currently no event to navigate backwards (or forwards) through the browser's history. I agree we should probably have one.

@goto-bus-stop
Copy link
Member

There's currently no event to navigate backwards (or forwards) through the browser's history. I agree we should probably have one.

is this necessary when you can do history.back()?

@bates64
Copy link
Contributor

bates64 commented Jan 31, 2018

We could mention history.back() in the choo docs where pushState is, and perhaps rename popState (major!!) to make it more obvious that it doesn't cause anything like pushState does.

@yoshuawuyts
Copy link
Member

@goto-bus-stop yeah, we probably should have such a method. Part of the event bus is to provide visibility as to which actions are being taken. Pretty great help for debugging haha.

@heyitsmeuralex a PR to the docs mentioning this would be grand!

@goto-bus-stop
Copy link
Member

yeah, we probably should have such a method. Part of the event bus is to provide visibility as to which actions are being taken. Pretty great help for debugging haha.

makes sense!

bates64 pushed a commit to bates64/nanochoo that referenced this issue Feb 7, 2018
@Powersource
Copy link

Note that this is still incorrectly documented on the website choojs/website#75

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

No branches or pull requests

5 participants