choojs / choo

:steam_locomotive::train: - sturdy 4kb frontend framework
https://choo.io/
MIT License
6.78k stars 595 forks source link

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

Closed timsim00 closed 6 years ago

timsim00 commented 6 years ago

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 commented 6 years ago

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

yoshuawuyts commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

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!

Powersource commented 5 years ago

Note that this is still incorrectly documented on the website https://github.com/choojs/website/issues/75