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

Fixes #48: add a prettify query button #64

Merged
merged 1 commit into from
Jan 27, 2016

Conversation

gre
Copy link
Contributor

@gre gre commented Nov 28, 2015

addresses #48

it adds a {} prettify button. this is implemented using graphql parse & print function: print(parse(this.state.query))

(pretty useful when you paste an obfuscated relay query and want to debug it)

screen shot 2015-11-28 at 11 14 35

on hover:

screen shot 2015-11-28 at 11 14 42

after clicking:

screen shot 2015-11-28 at 11 14 57

@skevy
Copy link
Contributor

skevy commented Nov 29, 2015

Awesome!!

@leebyron
Copy link
Contributor

Interesting! I think we may want to improve the button affordance, it's not clear that it is clickable. One significant downside of this prettify technique is that it eats comments. Obviously not a big deal for cleaning up obfuscated queries that have no comments, but something to consider.

@gre
Copy link
Contributor Author

gre commented Nov 30, 2015

I'm not sure how we can preserve comments. Sounds like something to fix on the graphql parser/printer side.

I have intentionally made the {} button looking secondary (less important than the ▶one), but actually I would be happy to implement any better UI if you have an idea on how to make it look more clickable (it already reacts on hover as you can see).
I feel that if using a circle or a square border will look weird next to the circle ▶button.

I've also inspired from the Web Console:
screen shot 2015-11-30 at 20 57 10

@vladar
Copy link

vladar commented Jan 23, 2016

That would be so useful with Relay

const { onClick } = this.props;
if (onClick) {
e.preventDefault();
onClick();
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: maybe just this.props.onClick()

@asiandrummer
Copy link
Contributor

Sorry it took a while to get to this @gre. I think this is a nice feature addition - although we have a known issue with comments being deleted, I think this will be very useful for folks working with reduced/minimized query string, especially in Relay land.

@skevy
Copy link
Contributor

skevy commented Jan 26, 2016

I'm so excited to have this in GraphiQL, @gre! :)

@gre
Copy link
Contributor Author

gre commented Jan 26, 2016

me too :)

@asiandrummer
Copy link
Contributor

@gre Could we squash the commits into one commit?
Let's merge this in and see if we could make an improvement in GraphQL parser/printer to preserve comments. Undo/redo within the editor should be able to mitigate the downside for the time being I think.

@gre
Copy link
Contributor Author

gre commented Jan 27, 2016

@asiandrummer are you sure I should squash my commit and lose the history? is this really the convention here? (merge should hide the local branch history no?)

@asiandrummer
Copy link
Contributor

@gre I was thinking we could only have a single or a couple commit related to the feature, for the readability of the commit history reasons. I was especially thinking we could include (or amend the message of) the 2nd commit if that's okay.

Also I think merge actually shows the local branch history (#15), but I might be missing something :p

@gre
Copy link
Contributor Author

gre commented Jan 27, 2016

this is squashed now

@asiandrummer
Copy link
Contributor

image
Citizen Hyo thanks you! I'm really excited for this feature.

asiandrummer added a commit that referenced this pull request Jan 27, 2016
Fixes #48: add a prettify query button
@asiandrummer asiandrummer merged commit 030d764 into graphql:master Jan 27, 2016
acao pushed a commit to acao/graphiql that referenced this pull request Jun 1, 2019
Update mocha to the latest version 🚀
acao pushed a commit to acao/graphiql that referenced this pull request Jun 5, 2019
This adds a protocol-compliant GraphQLConfig implementation, as well as an incomplete test suite for placeholder. Uses a new `[email protected]`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants