Skip to content

Fix Anchor Link triggers transition #361

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
Feb 5, 2020
Merged

Conversation

PragTob
Copy link
Collaborator

@PragTob PragTob commented Feb 5, 2020

Issue #4: Anchor Link Click triggers full page transition

Changes

  • added tests reproducing the problem and showing where it does and doesn't trigger
  • only trigger navigation if anything that is not the hash changed

Notes

  • included pry as it's really helpful to me

@@ -1,7 +1,7 @@
import Vue from 'vue/dist/vue.esm'
import axios from 'axios'
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wasn't used anywhere, in the file so I removed it. I hope I'm not unaware of some side effect this had, but the tests pass 🤷‍♂️


# Hacky/instable but easy way to set my custom App for this page
def set_app_class
@app_class = Apps::MyTestApp
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as the comment states, this is hacky but considering doing naming convetions, a new controller and appending to the routes etc. this seemed preferable to me.

@PragTob PragTob changed the title Fix 4 anchor link Fix Anchor Link triggers transition Feb 5, 2020
@jonasjabari jonasjabari merged commit 6a5003d into develop Feb 5, 2020
@PragTob PragTob deleted the fix-4-anchor-link branch February 5, 2020 17:44
@jonasjabari jonasjabari added this to the 0.7.4 milestone Feb 10, 2020
@jonasjabari jonasjabari mentioned this pull request Mar 9, 2020
5 tasks
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.

2 participants