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

Prefix operation ids with parent id #1245

Merged
merged 4 commits into from
Apr 12, 2022

Conversation

BBlowers
Copy link
Contributor

@BBlowers BBlowers commented Apr 24, 2020

Currently, operations that have multiple tags appear multiple times on the page as desired, however, their containing elements share the same id. This means when using the side menu to navigate the user is always taken to the earliest occurrence.

To demonstrate I've added the pet tag to the "Returns pet inventories by status" operation in this gif. Clicking on the "Returns pet inventories by status" operation in the store section takes the user to the operation under the pet section
redoc section links

A solution to this is to prefix operation ids with their parent's id.

@RomanHotsiy
Copy link
Member

@BBlowers thank you for your PR! Unfortunately, this is a breaking change. Old URLs will stop working.

Would you have some time add support for old operation/* URLs or to enable this behavior optionally by creating new Redoc option.

Let me know, I can pick this up.

@BBlowers
Copy link
Contributor Author

@RomanHotsiy good point.

Supporting both operation/* and parent/*/operation* makes the most sense to me. I'll update the PR asap

@BBlowers
Copy link
Contributor Author

BBlowers commented May 12, 2020

@RomanHotsiy I've added the operation/* format id to the OperationRow component which works with the scrolling system being used.

I've kept the changes to a minimum as I'm not too familiar with typescript.

@BBlowers
Copy link
Contributor Author

BBlowers commented Jun 4, 2020

@RomanHotsiy is this PR likely to get another look soon?

@RomanHotsiy
Copy link
Member

Hey @BBlowers,

Sorry for the radio silence. I will review and merge it this week. This looks good. I just have to test if it doesn't break our other tools.

@BBlowers
Copy link
Contributor Author

@RomanHotsiy How did the tests go? Is this okay to be merged?

@RomanHotsiy
Copy link
Member

Oh, sorry, not really. I haven't tested it yet.

Thanks for pinging. I am now adding it to the team sprint board so we prioritize it. If it is not merged mid-next week - feel free to ping me again!

Sorry, busy months for me :(

@RomanHotsiy
Copy link
Member

I just tested it out and there is some conflict with our internal tool. It should be fixed early next week and I will merge after.

@BBlowers
Copy link
Contributor Author

BBlowers commented Nov 3, 2020

any news?

@asauray
Copy link

asauray commented Dec 1, 2020

@RomanHotsiy Any update on this pr?

@altanozlu
Copy link

any news ?

@RomanHotsiy
Copy link
Member

Hi folks! Huge delay here but we'll merge it soon! We've finally got some time here.

It will be included in 2.0.0 release in about a month.

@RomanHotsiy RomanHotsiy added this to the v2.0 milestone Mar 28, 2022
@anastasiia-developer anastasiia-developer self-assigned this Apr 7, 2022
@anastasiia-developer
Copy link
Contributor

@BBlowers Hi! Thank you for your contribution to redoc.

@anastasiia-developer anastasiia-developer merged commit fd8917e into Redocly:master Apr 12, 2022
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.

6 participants