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

closes #953 add possibility to ovewrite graphiql html #954

Closed
wants to merge 3 commits into from

Conversation

lioman
Copy link

@lioman lioman commented May 26, 2020

If graphiql parameter of app is set to a str, we can use it to overwrite the built in template

classes starting with 'Test' produce a pytest warning
if they have a __init__ method.
Renaminig it because it is no test class
@lioman lioman changed the title #953 add possibility to ovewrite graphiql html closes #953 add possibility to ovewrite graphiql html May 26, 2020
@lioman
Copy link
Author

lioman commented May 26, 2020

closes #953

@lioman
Copy link
Author

lioman commented Jun 18, 2020

Not sure how to push this, or help with review. Would be nice to have this in starlette.

@erewok
Copy link
Contributor

erewok commented Aug 17, 2020

It seems like a potentially useful configurable parameter to add, but the type [bool, str] is one that I find confusing, especially in combination with the isinstance call later on: that variable is being coerced into a wide range of responsibilities. Consider how you would support, for instance, passing a str that represents possibly a template itself (such as the default in the code) or a filepath which could be retrieved and used as a template?

It may be helpful to look at other libraries as well. For instance, graphql-server has a config which for Flask offers the following options:

- graphiql: If True, may present GraphiQL when loaded directly from a browser (a useful tool for debugging and exploration).
- graphiql_version: The graphiql version to load. Defaults to "1.0.3".
- graphiql_template: Inject a Jinja template string to customize GraphiQL.
- graphiql_html_title: The graphiql title to display. Defaults to "GraphiQL".

@JayH5 JayH5 added the graphql label Sep 11, 2020
@JayH5
Copy link
Member

JayH5 commented Feb 5, 2021

Thank you for creating this pull request. We have decided to deprecate GraphQL support within Starlette itself so I am going to close this pull request. See #619.

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

3 participants