-
Notifications
You must be signed in to change notification settings - Fork 9.1k
Fixes #3596 - Deep linking issue with spaces #3661
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
Fixes #3596 - Deep linking issue with spaces #3661
Conversation
…596-permalink-encoding
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm able to break this implementation using the sample spec from #3631 (found at https://gist.githubusercontent.com/shockey/92386d99bac3962d4927e5c02fd43f37/raw/0a34e7de896370672e0a8984b49cef96b810d6d0/ui-3631.yaml):
Uncaught (in promise) DOMException: Failed to execute 'querySelector' on 'Document':
'#operations-Tag_with_special_characters?-post_resource'
is not a valid selector.
Since the ids on tags and operations will be used in a querySelector, we need to escape/encode any characters that are special in the context of a CSS selector.
My guess is that a string URL encoder will get us pretty far in that regard, but encoding needs to be balanced with readability of the resulting URL.
…ting the ID into the DOM. Escape the deep link path when querying for the DOM element on hash change.
@shockey I've update the PR to use the CSS.escape (via polyfill). Tested in Chrome, Safari, and Firefox with special characters and spaces. |
Thanks @owenconti, the changes look good. Can you add some tests for |
Approving this so it ships today. I'll create another ticket asking for tests 👍 |
Fixes #3596
Wrapped tags and operationIds in function that will replace spaces with underscores. I chose this approach because
swagger-js
is already replacing spaces with underscores for operationIds in theopId
function.