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

Scroll to anchor in router.scrollBehavior doesn't work #213

Closed
1 task done
kidonng opened this issue May 27, 2019 · 4 comments · Fixed by #214
Closed
1 task done

Scroll to anchor in router.scrollBehavior doesn't work #213

kidonng opened this issue May 27, 2019 · 4 comments · Fixed by #214
Assignees
Labels
🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt

Comments

@kidonng
Copy link
Contributor

kidonng commented May 27, 2019

Issuehunt badges

  • I confirm that this is an issue rather than a question.

Bug report

if (to.hash) {
let hash = to.hash
// CSS.escape() is not supported with IE and Edge.
if (
typeof window.CSS !== 'undefined' &&
typeof window.CSS.escape !== 'undefined'
) {
hash = '#' + window.CSS.escape(hash.substr(1))
}
try {
if (document.querySelector(hash)) {
// scroll to anchor by returning the selector
position = { selector: hash }
}
} catch (e) {
console.warn(
'Failed to save scroll position. Please add CSS.escape() polyfill (https://github.com/mathiasbynens/CSS.escape).'
)
}
}

The code attempts to scroll to the anchor if to.hash exists and the anchor can be found. But the scroll never happens.

Version

0.6.4

Steps to reproduce

about.md:

[Hello](./hello.md#world)

hello.md:

# World

Lorem ipsum...

Then click the link in about.md.

What is expected?

Page scrolled to the title World.

What is actually happening?

Page scrolled to the top (default behavior).

Other relevant information

  1. If to.hash only contains ASCII characters, the hash before and after escaping are the same. If it contains non-ASCII characters (e.g. CJK characters), escaping the hash will produce a selector matching nothing (assuming header's id is generated in standard behavior).

Possible solution: Use decodeURIComponent(to.hash) instead of window.CSS.escape(to.hash)

  1. Even if the selector is valid, document.querySelector(hash) still matches nothing, thus the scroll won't trigger. Chances are page is not loaded at that time.

Possible solution: Move the codition statement to a hook like afterPageLoad or use other methods (can only come up with setTimeout(condition, 0))

  • Result of running saber -v: cli.js/0.6.4 win32-x64 node-v12.3.1
  • Browser version (optional): Google Chrome 76.0.3800.0 (64-bit)
  • Is Saber a global or local install? Local
  • Which package manager did you use for the install? Yarn
  • Does this issue occur when all plugins are disabled? Yes
IssueHunt Summary

krmax44 krmax44 has been rewarded.

Sponsors (Total: $50.00)

Tips

@kidonng kidonng changed the title scroll to anchor in scrollBehavior doesn't work Scroll to anchor in router.scrollBehavior doesn't work May 27, 2019
@issuehunt-oss issuehunt-oss bot added the 💵 Funded on Issuehunt This issue has been funded on Issuehunt label May 27, 2019
@IssueHuntBot
Copy link

@IssueHunt has funded $50.00 to this issue.


@krmax44 krmax44 self-assigned this May 27, 2019
krmax44 added a commit to krmax44/saber that referenced this issue May 27, 2019
egoist pushed a commit that referenced this issue May 29, 2019
closes #213 

* fix: #213

* add scroll on initial load

* fix: initial page load scroll on Chromium

* remove css.escape

* remove unnecessary code

* avoid selector escaping issues
@issuehunt-oss issuehunt-oss bot added 🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt and removed 💵 Funded on Issuehunt This issue has been funded on Issuehunt labels May 29, 2019
@IssueHuntBot
Copy link

@egoist has rewarded $35.00 to @krmax44. See it on IssueHunt

  • 💰 Total deposit: $50.00
  • 🎉 Repository reward(20%): $10.00
  • 🔧 Service fee(10%): $5.00

@kidonng
Copy link
Contributor Author

kidonng commented May 29, 2019

Scrolling to id containing URI encoded characters still has issue 😢

image

First two triggered by clicking the heading anchor (not encoded and working), second two using browser back/forward (encoded and not working).

We should put a const hash = decodeURIComponent(to.hash) here:

if (to.hash) {
if (document.getElementById(to.hash.substr(1))) {
// scroll to anchor by returning the selector
position = { selector: to.hash }

@egoist
Copy link
Collaborator

egoist commented May 29, 2019

@kidonng fixed in 0.6.7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants