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

Allow scrollBehavior selectors to match ids with unescaped CSS special characters (feature request #3008) #3009

Closed
wants to merge 3 commits into from

Conversation

theprojectsomething
Copy link

As per the title and the original feature request #3008 this PR means a wider range of selectors are passed to document.getElementById, resulting in more straightforward use of scrollBehavior with e.g. to.hash

previously a scrollBehavior selector was only checked for ^#\d when determining whether to use getElementById or querySelector for matching; this would not work as intended for chained queries (e.g. `vuejs#9, .main`) or those with CSS special characters (e.g. `#one/two`).
@@ -121,14 +121,16 @@ function isNumber (v: any): boolean {
return typeof v === 'number'
}

const hashStartsWithNumberRE = /^#\d/
const selectorLooksLikeIdRE = /^#[^, ]+$/
Copy link
Member

Choose a reason for hiding this comment

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

why any character but , and ?

Copy link
Author

Choose a reason for hiding this comment

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

@posva a , or suggests a complex selector (either nested or an OR). Technically an id selector could contain an , but the above provides the best balance of simplicity vs. coverage without breaking changes. Consider:

// Simple selector (passed to getElementById):
#one
#1
#one/two

// Complex selector (passed to querySelector): 
#one .two
#one, .two
#one\,two // selects id="one,two"

Did you have any suggestions?

Copy link
Author

Choose a reason for hiding this comment

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

Hi @posva any update on this PR?

@posva
Copy link
Member

posva commented May 26, 2020

Thanks for this but I think we need to go in a different direction: #3008 (comment)

@posva posva closed this May 26, 2020
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.

None yet

2 participants