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

Run GraphQL query based on cursor position. #990

Merged
merged 4 commits into from Jun 27, 2018

Conversation

@bwlt
Copy link
Contributor

bwlt commented Jun 17, 2018

If there are multiple operations in the GraphQL query it automatically choose
which operation to run based on the cursor position.

If there are multiple operations in the GraphQL query it automatically choose
which operation to run based on the cursor position.
@welcome

This comment has been minimized.

Copy link

welcome bot commented Jun 17, 2018

💖 Thanks for opening this pull request! 💖

To help make this a smooth process, please be sure you have first read the
contributing guidelines.

@adnsio
adnsio approved these changes Jun 17, 2018
@gschier

This comment has been minimized.

Copy link
Contributor

gschier commented Jun 18, 2018

This is a very well done PR, thanks!

I will also leave some actual review comments but I just wanted to have a brief high-level discussion about this feature first.

While I do think this feature is useful (and I know it's part of GraphiQL), it seems like there is potential to cause confusion since there's no obvious indication that the cursor position is controlling the selected query.

A quick improvement could be to highlight the selected query name based on the currently active OperationName. I'm perfectly happy making this (or similar) improvement after merging this PR. However, if you felt like adding this yourself that would be awesome too.

Thoughts?

@bwlt

This comment has been minimized.

Copy link
Contributor Author

bwlt commented Jun 19, 2018

Ok, how to highlight? (in terms of style)
I thought on these two alternatives:

  • the selected query have syntax color
    screen shot 2018-06-19 at 14 16 11

  • the selected query have a bolder operation name
    screen shot 2018-06-19 at 14 16 35

(I'm not good on design stuff 😅)

@gschier

This comment has been minimized.

Copy link
Contributor

gschier commented Jun 20, 2018

Ooo I like grayscaling everything else but I'm not sure how difficult that is to implement. If it's not easy, I would also be fine with using Codemirror's markText function to highlight the background of the query name.

Here's a code snippet of how it works.

editor.markText({line: 6, ch: 26}, {line: 6, ch: 42}, {className: "styled-background"});

Setting up the precise colors to use may be tricky since it has to work with the Insomnia theme system. But, if you want to add the basic implementation I can help fine-tune it if needed.

@gschier

This comment has been minimized.

Copy link
Contributor

gschier commented Jun 20, 2018

@bwlt

This comment has been minimized.

Copy link
Contributor Author

bwlt commented Jun 21, 2018

@gschier

This comment has been minimized.

Copy link
Contributor

gschier commented Jun 21, 2018

Awesome! I'm looking forward to it 😄

Let me know if you need any help! 👍

If the current operation name gets deleted the new current operation
is set as fallback to the first operation name.
This refactor includes a `_documentAST` property on GraphQLEditor component,
so it is locally available (it avoids multiple graphql `parse` function calls).
Also a _queryEditor reference is added to the component.
@@ -21,4 +21,10 @@
.graphql-editor__query .cm-attribute {
color: var(--color-info);
}

&__disabled-definition {
&&& {

This comment has been minimized.

Copy link
@bwlt

bwlt Jun 22, 2018

Author Contributor

@gschier sorry, this is a dirty trick to increase the specificity (always better than !important, inspired from styled-components)

@bwlt bwlt closed this Jun 22, 2018
@bwlt bwlt reopened this Jun 22, 2018
@gschier gschier merged commit 29d13ce into Kong:develop Jun 27, 2018
0 of 2 checks passed
0 of 2 checks passed
continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
@welcome

This comment has been minimized.

Copy link

welcome bot commented Jun 27, 2018

Congrats on merging your first pull request! 🎉🎉🎉 You're helping make Insomnia awesome! 🙌

@bwlt bwlt deleted the bwlt:feature/graphql-choose-operation-name branch Jun 28, 2018
luizmariz pushed a commit to luizmariz/insomnia that referenced this pull request Jan 22, 2020
* Run GraphQL query based on cursor position.

If there are multiple operations in the GraphQL query it automatically choose
which operation to run based on the cursor position.

* Highlight current operation

* Refactor. Prevent some query change issues.

If the current operation name gets deleted the new current operation
is set as fallback to the first operation name.
This refactor includes a `_documentAST` property on GraphQLEditor component,
so it is locally available (it avoids multiple graphql `parse` function calls).
Also a _queryEditor reference is added to the component.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.