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

Allow typing event arguments for autocomplete #1180

Merged
merged 11 commits into from
Oct 20, 2022
Merged

Allow typing event arguments for autocomplete #1180

merged 11 commits into from
Oct 20, 2022

Conversation

Janpot
Copy link
Member

@Janpot Janpot commented Oct 19, 2022

Context: #1105 is blocked on supplying autocomplete to arguments passed to event handlers

This PR adds an option arguments to createComponent argument type of 'event' that allows specifying a TS interface, to be used for autocomplete. A special type ThisComponent will be available that allows you to use live values of other props. See https://github.com/mui/mui-toolpad/pull/1180/files#diff-b355b8b4105dc88c343f9bef59352a13b361f2f4152c6b77d58bbb6815361f3eR384 for an example

Additionally I had to tweak the global scope explorer to allow for displaying additional global variables. Currently they don't have any extra information on their content.

In order for that to work I had to change a few things and it now looks like:

Screenshot 2022-10-19 at 14 24 21

I propose we avoid expanding the scope of this PR and defer further refinement to #497

@render
Copy link

render bot commented Oct 19, 2022

@Janpot Janpot requested review from bytasv and bharatkashyap and removed request for bytasv October 19, 2022 12:27
@Janpot Janpot added the core Infrastructure work going on behind the scenes label Oct 19, 2022
@oliviertassinari oliviertassinari requested a deployment to event-typing - toolpad-db PR #1180 October 19, 2022 12:36 — with Render Abandoned
@Janpot
Copy link
Member Author

Janpot commented Oct 19, 2022

@bytasv Was running into limitations with global scope explorer. My fix also removes the nodeRenderer (#1165 (comment))

@Janpot Janpot marked this pull request as ready for review October 19, 2022 12:49
Copy link
Contributor

@bytasv bytasv left a comment

Choose a reason for hiding this comment

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

Looks good ✅

Few minor suggestions to consider:

  1. with "virtual scroll" initially UI looks a bit "broken" - maybe it would be worth to consider something that could hint this is scrollable, i.e. using some scroll component that always shows "virtual type" scrollbars. Alternatively maybe add a shadow on the right side which could be hidden upon first scroll

image

  1. I'm wondering if we need all that space for expression editor, maybe we could make scope explorer at least a bit wider (I tried setting it to 250px) and also reduce font size a bit so that more info fits into the same area (I tried with 12px)

image

  1. Maybe we could also render object's content below it's name, so we would get even more space upon expanding it (might not need to make width of the container itself bigger)

image

@Janpot
Copy link
Member Author

Janpot commented Oct 19, 2022

  1. I added borders which seems to be more the aesthetic of the theme
  2. made it 250, but we should probably just make this a resizable split pane at some point
  3. we're a bit constrained by what's possible with react-inspector. I've added the object inspector under the name for now. We may be able to do something a bit better if they fix [Bug] Setting OBJECT_PREVIEW_OBJECT_MAX_PROPERTIES to 0 doesn't work storybookjs/react-inspector#163. But not holding my breath for that 🙂

@Janpot Janpot merged commit 70064bf into master Oct 20, 2022
@Janpot Janpot deleted the event-typing branch October 20, 2022 12:24
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants