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

monaco-graphql tabs through intellisense too aggressively #2121

Closed
jeffsee55 opened this issue Dec 15, 2021 · 4 comments · Fixed by #2488
Closed

monaco-graphql tabs through intellisense too aggressively #2121

jeffsee55 opened this issue Dec 15, 2021 · 4 comments · Fixed by #2488

Comments

@jeffsee55
Copy link

When you enter tab so select an intellisense option, it immediately drops you into the nested properties. This is different from codemirror graphql where it just puts you at the end of the word. This is mainly an issue when you have arugments you'd like to provide (especially when they're required)

https://www.loom.com/share/e20881c74d2f49f4858d508539c79a7a

@acao
Copy link
Member

acao commented Dec 15, 2021

first feedback about this new feature 🎂 ! recently introduced this to graphql-language-service-server and vscode-graphql as well

this is very helpful feedback! I hear this and could see how this would get annoying.

it's just so temptingly easy to add this on completion because of command and insertText in monaco.languages.CompletionItem. but also this seems like bad DX in retrospect, and makes field arguments more difficult to use

a few easier options for now:

  1. make this an opt-in feature
  2. only expand selection set for leaf fields without any arguments?
  3. only expand selection set for leaf fields with required arguments? (more expensive, is that even useful if you want to provide arguments though?)

more difficult, may take longer:

  1. if the field type has required arguments, expand the selection set automatically below as an additionalInsertText, but move the cursor to complete for the first argument. like this:
query {
  chosenField(|) {
    #selection set expanded, but cursor is in arguments
  }
}
  1. choose to use an editor command/keybinding for this sort of behaviour similarly to graphiql? (I think this may be the best option to avoid unexpected behavior for now). another or perhaps complementary option along this route would be a code action lightbulb that expands a selection set if you've left one un-expanded

definitely will take longer:

  1. i have a WIP from this effort for required field and argument completion as well, but there are still some open DX questions and issues

also while we're here, can I ask - would it be helpful for it to be clearer in suggestions when fields or arguments are required? I've also thought of modifying the suggestion sort to boost required fields or arguments to the top of the suggestion list.

@jeffsee55
Copy link
Author

These are really interesting options! It's really hard to say because this is such a feel thing. I'd rather step through a bit more slowly because so much of GraphQL is about discovery 🤔 . In general, 5 sounds like the best option to me.

would it be helpful for it to be clearer in suggestions when fields or arguments are required?

How would it be more clear? I see the String! in the hint box but maybe there's more to it?

Screen Shot 2021-12-14 at 6 34 27 PM

@acao
Copy link
Member

acao commented Dec 15, 2021

oh great! I did get the type printing fully there. couldn't recall offhand

@acao
Copy link
Member

acao commented Oct 27, 2022

finally about to release a fix for this. I'm sure you and I aren't the only ones annoyed by the outcome of this shoegun insertText experiment, haha

#2488

it will become an opt-in experimental feature, and your feedback and others will help us make this a much more well-rounded experience in the future, when it's ready. a custom keyboard shortcut for this with support for required arguments is a must have!

@acao acao closed this as completed in #2488 Nov 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants