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

refactor(gatsby-remark-code-repls): move config per provider #12936

Merged
merged 20 commits into from
Jun 25, 2019

Conversation

deblasis
Copy link
Contributor

Description

Currently it's possible to upload multiple files only by using CodeSandbox.

This PR adds an option that allows adding CSS files to CodePen examples by setting the css property in the payload object.

Ref: https://blog.codepen.io/documentation/api/prefill/

Matching CSS means that if the user is linking to example.js and the option is set to true, if example.css is present in the same directory, its content is sent to CodePen.

The default behaviour is left untouched
If the option is set to true and the matched file doesn't exist, nothing changes
tests included

Related Issues

This is based upon the suggestions in PR #12110 and it includes the breaking API changes

options: {
....
// Include CSS with matching name.
// This option only applies to REPLs that support it (eg Codepen).
// If set to `true`, when specifying `file1.js` as example file,
// it will try to inject the CSS in `file1.css` if the file exists,
// otherwise the default behaviour is preserved
includeMatchingCSS: false,
}
confict resolved

Signed-off-by: Alessandro De Blasis <[email protected]>
Signed-off-by: Alessandro De Blasis <[email protected]>
Signed-off-by: Alessandro De Blasis <[email protected]>
Signed-off-by: Alessandro De Blasis <[email protected]>
…ring

This is based upon suggestions in PR gatsbyjs#12110 review.
Since there is a breaking change in the API, the version has been ⬆

Signed-off-by: Alessandro De Blasis <[email protected]>
Signed-off-by: Alessandro De Blasis <[email protected]>
@DSchau DSchau added the status: awaiting author response Additional information has been requested from the author label Apr 8, 2019
Copy link
Contributor

@wardpeet wardpeet left a comment

Choose a reason for hiding this comment

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

This is really great! thanks for the refactoring. I'm going to test this one

@deblasis
Copy link
Contributor Author

This is really great! thanks for the refactoring. I'm going to test this one

Awesome! You are welcome, happy to contribute back a little bit :)

Apparently we merged to master the previous version that added the feature to include the matching
css in the folder if present. This version is the refactored one which also improves the API surface
by scoping the options depending on the REPL provider

BREAKING CHANGE: The API surface is not backward compatible anymore as the options have been scoped
per-provider as suggested during code-review gatsbyjs#12110
It will be incremented on publish, make sure it's a major version

BREAKING CHANGE: API changes (read PR gatsbyjs#12936)
@deblasis deblasis requested a review from wardpeet April 27, 2019 15:35
@deblasis
Copy link
Contributor Author

Hi @wardpeet , the PR that adds the feature was merged, this one is the API refactoring one. Let's remember to publish a major version since the API changed as suggested 😄

@LekoArts LekoArts added status: awaiting reviewer response A pull request that is currently awaiting a reviewer's response and removed status: awaiting author response Additional information has been requested from the author labels May 27, 2019
@LekoArts
Copy link
Contributor

Hey @wardpeet 👋 Seems like this needs final approval, want to have a look? Thanks!

@deblasis deblasis requested a review from a team as a code owner June 25, 2019 09:41
@wardpeet wardpeet self-assigned this Jun 25, 2019
@wardpeet
Copy link
Contributor

I was testing this plugin on reactjs.org site and it seemed like it wasn't working great on windows machines so I made it work. Will publish when everything is green on new major

Copy link
Contributor

@wardpeet wardpeet left a comment

Choose a reason for hiding this comment

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

Thanks for you hard work & your patience! <3 The code is looking great!

@wardpeet wardpeet changed the title feat(gatsby-remark-code-repls): include matching css option v3 refactor(gatsby-remark-code-repls): move config per provider Jun 25, 2019
@wardpeet wardpeet merged commit 2beb0ec into gatsbyjs:master Jun 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: awaiting reviewer response A pull request that is currently awaiting a reviewer's response
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants