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

fix: cmdk key bindings clash with Supabase Studio’s built-in GraphiQL… #18898

Conversation

PurnenduMondal
Copy link

@PurnenduMondal PurnenduMondal commented Nov 12, 2023

To fix the bug, I added onFocus and onBlur props to the GraphiQL QueryEditor component. These props are used to set the focus state of the QueryEditor. Then I created a new context named GraphiQLQueryEditorFocusContext to store and access the focus state of the QueryEditor globally and then I fetched the isGraphiQLEditorOnFocus state in CommandMenuWrapper and passed it as a prop to the CommandMenuProvider, with the help of this prop I am restricting the command menu to open when the GraphiQL QueryEditor is in focus so that cmd / shortcut can only be used to comment out the code in the QueryEditor.

Issue: 18269

@PurnenduMondal PurnenduMondal requested a review from a team as a code owner November 12, 2023 14:54
Copy link

vercel bot commented Nov 12, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
ui-storybook ⬜️ Ignored (Inspect) Visit Preview Nov 14, 2023 5:50am

Copy link

vercel bot commented Nov 12, 2023

Someone is attempting to deploy a commit to the Supabase Team on Vercel.

A member of the Team first needs to authorize it.

@alaister
Copy link
Member

Hey @PurnenduMondal,

Thanks for your PR!

I'm worried that this approach adds both user-land complexity (getting a different cmd k menu than you're expecting) and code complexity (the existing cmd k menu has to worry about GraphiQL now).

So, what do you think about just modifying the existing GraphiQL shortcut? Could be something like cmd option k?

@alaister alaister self-assigned this Nov 13, 2023
@PurnenduMondal
Copy link
Author

Hi @alaister

Thank you for taking the time to review the PR, and I appreciate your feedback.

Modifying the existing GraphiQL shortcut, such as using cmd option k, seems like a reasonable suggestion. This approach could help avoid conflicts with the default cmd k menu and minimize disruptions for users. I will try it out and let you know.

@PurnenduMondal
Copy link
Author

Hi @alaister

I am currently experiencing some issues while running the master branch locally. In order to run it, I am following these steps:

  1. Running all the containers except the studio.
  2. Running the studio using 'npm run dev'.

However, when accessing the GraphiQL Editor page at http://localhost:8082/project/default/database/graphiql, it always returns a 404 error in response to these API calls:

GET http://localhost:8082/api/projects-resource-warnings
GET http://localhost:8082/api/integrations/default-org-slug?expand=true

I believe this is causing the page to remain in a 'Loading' state. Could you please let me know if I am doing something wrong and what I can do to resolve this issue?

Thank you.

@alaister
Copy link
Member

Sorry, we're moving GraphiQL around a bit. Please make sure your branch is up to date, then visit http://localhost:8082/dashboard/project/default/api/graphiql.

If it's still not working, I'll dig a bit deeper!

@PurnenduMondal
Copy link
Author

PurnenduMondal commented Nov 14, 2023

Hi @alaister

My branch is up to date but It's still returning a 404 page at http://localhost:8082/dashboard/project/default/api/graphiql.

I have also attempted the following URL:
http://localhost:8082/project/default/api/graphiql.
However, after a prolonged loading period, it throws the following error:
FATAL ERROR: Reached heap limit. Allocation failed - JavaScript heap out of memory.

System information:
OS: Windows 11,
Browser: Chrome 119.0.6045.124.

@saltcod
Copy link
Contributor

saltcod commented Aug 8, 2024

Hey @PurnenduMondal sorry for the back and forth on this PR. At the moment, we're considering moving GraphiQL to a different part of the Dashboard which might indirectly fix this with no additional code. I'm going to mark this as closed, but appreciate your effort here!

@saltcod saltcod closed this Aug 8, 2024
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.

3 participants