Skip to content

Consider exposing minimal static playground #66

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

Merged
merged 5 commits into from
Apr 28, 2020
Merged

Consider exposing minimal static playground #66

merged 5 commits into from
Apr 28, 2020

Conversation

jorgebay
Copy link
Contributor

@jorgebay
Copy link
Contributor Author

Could help write the documentation and getting users started...

@mpenick
Copy link
Contributor

mpenick commented Apr 15, 2020

This is so cool! 💯

@mpenick mpenick linked an issue Apr 15, 2020 that may be closed by this pull request
4 tasks
@mpenick
Copy link
Contributor

mpenick commented Apr 15, 2020

We should probably provide a way to disable it. Also, if not on a single keyspace the default URL should start on something other than /graphql,as a stop gap for something else maybe choose the first keyspace?

@jorgebay
Copy link
Contributor Author

provide a way to disable it

+1

maybe choose the first keyspace?

+1

@jorgebay jorgebay added enhancement New feature or request and removed discussion labels Apr 21, 2020
@jorgebay jorgebay marked this pull request as draft April 21, 2020 15:27
@jorgebay jorgebay marked this pull request as ready for review April 22, 2020 10:44
@jorgebay jorgebay requested a review from mpenick April 22, 2020 10:44
Copy link
Contributor

@mpenick mpenick left a comment

Choose a reason for hiding this comment

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

Nice! Just a quick question.

cmd/server.go Outdated
// Get the first POST GraphQL route
for _, route := range routes {
if route.Method == http.MethodPost && strings.HasPrefix(route.Pattern, rootPath) {
path = route.Pattern
Copy link
Contributor

Choose a reason for hiding this comment

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

For me this always defaults to: http://localhost:8080/graphql-schema, which is fine. Is that intended or should this point to a keyspace?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It happens because /graphql-schema starts with /graphql ...

it's not intended 🤦 , I'll fix it.

@jorgebay
Copy link
Contributor Author

I'll rebase this one once #94 is merged, as default endpoint path should be path.Join(rootPath, aValidKeyspace)

@jorgebay
Copy link
Contributor Author

I've rebased against master and fixed the default endpoint url issue.

There was an issue on the latest version of the playground that included a } character in the output and messed up the layout (try minimal.html locally to see it). I've fixed it by pointing a specific version of the files in the cdn.

Also, we have to look out for upcoming changes related to this: https://foundation.graphql.org/news/2020/04/03/web-based-graphql-ides-for-the-win-how-why-playground-graphiql-are-joining-forces/

@jorgebay jorgebay requested a review from mpenick April 27, 2020 10:03
@mpenick
Copy link
Contributor

mpenick commented Apr 27, 2020

Also, we have to look out for upcoming changes related to this: https://foundation.graphql.org/news/2020/04/03/web-based-graphql-ides-for-the-win-how-why-playground-graphiql-are-joining-forces/

Alright! The best of both world hopefully.

@jorgebay jorgebay merged commit 6cb1786 into master Apr 28, 2020
@jorgebay jorgebay deleted the playground branch April 28, 2020 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Getting started documentation with examples
2 participants