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

fix($location): back-button not triggering $locationChangeStart #2206

Closed
wants to merge 1 commit into from
Closed

fix($location): back-button not triggering $locationChangeStart #2206

wants to merge 1 commit into from

Conversation

quazzie
Copy link
Contributor

@quazzie quazzie commented Mar 22, 2013

$locationChangeStart does not get broadcasted when pressing the back-button.

bug example : http://plnkr.co/edit/NycOKi?p=preview

Closes #2109

@geddski
Copy link
Contributor

geddski commented Mar 22, 2013

LGTM. I've verified that this fixes the $location bug:
http://plnkr.co/edit/NycOKi?p=preview

nicely done @quazzie. Would you mind squashing your two commits into one? I can walk you through that if you'd like.

@quazzie
Copy link
Contributor Author

quazzie commented Mar 22, 2013

Please describe how and I'll do it. But if it requires anything other than a browser and github it will have to wait until monday. I'm away and don't have access to computer.

@geddski
Copy link
Contributor

geddski commented Mar 22, 2013

Yeah you'll need a computer. I'd do it for you but it's a good thing for you to learn. First read up on rebasing. Then give this a shot and let me know how it goes:

$ git rebase -i 23abb99. NOTE: 23abb99 is the commit right before your two commits.

Your terminal will now show you this:
image

Now on line 2 change the pick babcd38 change test to squash babcd38 change test.
Save by hitting :x enter.

Now you'll see:

image

Now comment out the second commit message. So change test becomes #change test.
Save again with :x enter.

That's it, now git will have combined your two commits into one, with a single commit message. This keeps the git repository clean and easier to see changes. Now just push your branch up to github (might need to use the -f flag) and this pull request will update. Normally it's a bad idea to rebase anything that's public, but a pull request to another project is usually ok.

Let me know if you have any questions.

$locationChangeStart does not get broadcasted when pressing the back-button.

Closes #2109
@quazzie
Copy link
Contributor Author

quazzie commented Mar 25, 2013

Done. Thanks for the help!

@geddski
Copy link
Contributor

geddski commented Apr 4, 2013

@IgorMinar take a look at this fix when you get a sec.

@petebacondarwin
Copy link
Member

  • Contributor signed CLA now or in the past (if you just signed, leave a comment here with your real name)
  • PR doesn't introduce new api
  • PR doesn't contain a breaking change
  • PR contains unit tests
  • PR contains e2e tests (if suitable)
  • PR contains documentation update (if suitable)
  • PR passes all tests on Travis (sanity)
  • PR passes all tests on ci.angularjs.org (cross-browser compatibility)
  • PR is rebased against recent master
  • PR is squashed into one commit per logical change
  • PR's commit messages are descriptive and allows us to autogenerate release notes (required commit message format)
  • All changes requested in review have been implemented

@quazzie
Copy link
Contributor Author

quazzie commented May 1, 2013

I signed cla a long time ago.

@petebacondarwin
Copy link
Member

Looks good to me: http://plnkr.co/edit/9KGLEwQlftGOXr9eQFUA?p=preview

@petebacondarwin
Copy link
Member

Landed as dc9a580. Thanks

@quazzie quazzie deleted the backbutton-locationchangestart branch May 2, 2013 06:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cancelling routes
3 participants