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

feat: add ability to override code examples using x-codeSamples #2574

Merged
merged 12 commits into from May 21, 2024

Conversation

weyert
Copy link
Contributor

@weyert weyert commented Apr 25, 2024

Elements Default PR Template

In general, make sure you have: (check the boxes to acknowledge you've followed this template)

Updates the RequestSamples and TryItWithExamples-components to support the x-codeExamples vendor extension on a http operation schema.

refs #2453

@weyert weyert requested a review from a team as a code owner April 25, 2024 15:20
@weyert weyert requested a review from billiegoose April 25, 2024 15:20
Copy link

netlify bot commented Apr 25, 2024

Deploy Preview for stoplight-elements ready!

Name Link
🔨 Latest commit ec84034
🔍 Latest deploy log https://app.netlify.com/sites/stoplight-elements/deploys/664b84291ea50c0008154970
😎 Deploy Preview https://deploy-preview-2574--stoplight-elements.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Apr 25, 2024

Deploy Preview for stoplight-elements-demo ready!

Name Link
🔨 Latest commit ec84034
🔍 Latest deploy log https://app.netlify.com/sites/stoplight-elements-demo/deploys/664b8429842de70008d51c84
😎 Deploy Preview https://deploy-preview-2574--stoplight-elements-demo.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@mnaumanali94
Copy link
Contributor

@weyert Hey! Thank you for the contribution. Were you going for x-codeexamples or was it supposed to be x-codesamples?

x-codesamples is generally what we see being used in the industry.

@weyert
Copy link
Contributor Author

weyert commented Apr 26, 2024

Oh sorry, my bad. I am happy to update it. You are totally right.
I am not sure if we would like to remap support mappings from other views (E.g. redly) to the values that Elements is expecting?

@brendarearden
Copy link
Contributor

I am not sure if we would like to remap support mappings from other views (E.g. redly) to the values that Elements is expecting?

I am not sure what you mean by this? is redly using a different overriding code example x extension?

@weyert
Copy link
Contributor Author

weyert commented Apr 26, 2024

I am not sure if we would like to remap support mappings from other views (E.g. redly) to the values that Elements is expecting?

I am not sure what you mean by this? is redly using a different overriding code example x extension?

Sorry, I meant Redocly and others might use a different naming convention for the language, e.g. shell vs bash etc. I think we are fine for Read.me version of doing this, though.

@brendarearden
Copy link
Contributor

Sorry, I meant Redocly and others might use a different naming convention for the language, e.g. shell vs bash etc. I think we are fine for Read.me version of doing this, though.

Gotcha. We don't need to do the remapping as part of this, but would you open a separate issue for the remapping?

@weyert
Copy link
Contributor Author

weyert commented May 3, 2024

Gotcha. We don't need to do the remapping as part of this, but would you open a separate issue for the remapping?

Sure, happy to do that.

Copy link
Contributor

@brendarearden brendarearden left a comment

Choose a reason for hiding this comment

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

This looks good - I tested out the storybook examples and it appears to be working!

Copy link
Contributor

@brendarearden brendarearden left a comment

Choose a reason for hiding this comment

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

Actually, after futher testing, it looks like the state is getting stuck when going to select different code samples

@weyert
Copy link
Contributor Author

weyert commented May 16, 2024

Hmm, bummer, wondering if it's incorrect management of the isStale-variable?

@daniel-white daniel-white force-pushed the ticket-code-example-overrides branch from 2db96f3 to ec84034 Compare May 20, 2024 17:11
@weyert
Copy link
Contributor Author

weyert commented May 20, 2024

Don't want to sound rude but I would prefer Stoplight's resources would be spend on my other PR #2545 which would allow me to stop using my fork of Elements. This PR was just a nice improvement.

@brendarearden
Copy link
Contributor

brendarearden commented May 20, 2024

@weyert this PR overlapped with a feature request from our product team, so it was a higher priority in the backlog. I appreciate you letting us know though and we will make sure your other PR is moved up in priority! hopefully we will be able to get you unforked soon!

Copy link
Contributor

@brendarearden brendarearden left a comment

Choose a reason for hiding this comment

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

looks good!

@brendarearden brendarearden merged commit 7a06aec into stoplightio:main May 21, 2024
7 checks passed
@weyert
Copy link
Contributor Author

weyert commented May 21, 2024

Thank you for merging :)

@weyert weyert deleted the ticket-code-example-overrides branch May 21, 2024 15:26
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

4 participants