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

Query panel improvements #2997

Open
1 of 5 tasks
bharatkashyap opened this issue Dec 14, 2023 · 0 comments
Open
1 of 5 tasks

Query panel improvements #2997

bharatkashyap opened this issue Dec 14, 2023 · 0 comments
Assignees
Labels
core Infrastructure work going on behind the scenes design: ui Design design: ux Design scope: toolpad-studio Abbreviated to "studio"

Comments

@bharatkashyap
Copy link
Member

bharatkashyap commented Dec 14, 2023

Collecting tasks be taken up once #2393 is merged

Bugs

Code

  • packages/toolpad-app/src/toolpad/AppEditor/NodeMenu.tsx

    const isAction = React.useMemo(() => {
        if (latestDeletedNode?.type === 'query' && latestDeletedNode?.attributes?.mode === 'mutation') {

    We can likely abstract this in a function and reuse in the filter where we split queries into "queries" and "actions". It's probably also not necessary to memoize. The memoization call is going to be more computationally expensive than two nested property lookups and a boolean operation.

Nomenclature

  • packages/toolpad-app/src/runtime/ToolpadApp.tsx

       let kind: 'query' | 'action' = 'query';
    • either we call it "action" everywhere and just do a translation to "mutation", right before we read/write to a file
    • or we call it "mutation" everywhere and name it "action" just right before we display in the UI

    The reason is that this prevents us from mismatching and constant translation all around the codebase. Ideally if the rename is done in a large PR, you start with the latter, just to avoid introducing many unnecessary chamges to unrelated code. Then in a separate PR update the codebase to the former.

UI/UX

  • The "Run" and "Save" button locations and types seem fine to me, but design-wise it feels like they need more vertical space around them. Not sure what the best way to improve that would be though.

    Screenshot 2023-11-23 at 16 37 41
  • Creating a new file in a custom function doesn't let us name the file, right? And the button at the bottom does not take the full-width of the popover so that may feel a bit weird. We can maybe improve the UX here later!

    Screenshot 2023-11-23 at 16 37 34
@bharatkashyap bharatkashyap added bug 🐛 Something doesn't work design: ui Design design: ux Design labels Dec 14, 2023
@bharatkashyap bharatkashyap self-assigned this Dec 14, 2023
@bharatkashyap bharatkashyap added core Infrastructure work going on behind the scenes and removed bug 🐛 Something doesn't work labels Dec 14, 2023
@prakhargupta1 prakhargupta1 added the scope: toolpad-studio Abbreviated to "studio" label Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Infrastructure work going on behind the scenes design: ui Design design: ux Design scope: toolpad-studio Abbreviated to "studio"
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants