Skip to content

Breadcrumbs replace state #588 #1093

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

Merged
merged 4 commits into from
Oct 17, 2017
Merged

Breadcrumbs replace state #588 #1093

merged 4 commits into from
Oct 17, 2017

Conversation

HeroProtagonist
Copy link
Contributor

resolves #588

  • Pulled out logic to fill a history function so it could be reused with replaceState
  • Updated test, by removing the window.onpopstate(); line it tests both filled history functions. Is that test sufficient?

@kamilogorek
Copy link
Contributor

Looks good, thanks!

Can you change the name of the test to include replaceState as well? Right now it only says

should record history.[pushState|back] changes as navigation breadcrumbs'

Can you also rebase it on top of master branch? It should fix the Travis issue.

@HeroProtagonist
Copy link
Contributor Author

@kamilogorek Updated the test name and did the rebase, but it seems travis still failed on sauce labs (╯°□°)╯︵ ┻━┻

image

@kamilogorek
Copy link
Contributor

It should be if [ "$TRAVIS_SECURE_ENV_VARS" == "true" ]; then npm run test:ci; else exit 0; fi

(╯°□°)╯︵ ┻━┻

I'll pull those changes and test them anyway. Thanks! :)

@kamilogorek kamilogorek merged commit 21a5484 into getsentry:master Oct 17, 2017
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

Successfully merging this pull request may close these issues.

Breadcrumbs should record replaceState as nav events
2 participants