mui / toolpad

Toolpad: Full stack components and low-code builder for dashboards and internal apps.
https://mui.com/toolpad/
MIT License
1.27k stars 283 forks source link

Page hierarchy explorer doesn't show element properties when they're not named "children" #3117

Open Janpot opened 9 months ago

Janpot commented 9 months ago

Steps to reproduce

Link to live example: (required)

Steps:

  1. Create a custom component:

    import * as React from 'react';
    
    import { createComponent } from '@mui/toolpad/browser';
    
    export interface MyComponentProps {
      notChildren: React.ReactNode;
    }
    
    function MyComponent({ notChildren }: MyComponentProps) {
      return <div>hello:{notChildren}</div>;
    }
    
    export default createComponent(MyComponent, {
      argTypes: {
        notChildren: {
          type: 'element',
          control: { type: 'slots' },
        },
      },
    });
  2. Drag into the page, and drag a button component inside of the "Drop component here" area.
  3. Check the hierarchy explorer

Current behavior

The button doesn't appear in the hierarchy explorer

Expected behavior

The button appears in the hierarchy explorer. Just like it does when you replace notChildren the custom component with children

Context

Noticed here. It looks like to solve this correctly, we need to signal in our dom when something is an element/template, independently from the components. I'm trying to introduce an { $$element: {...} } bindable property in the app dom and replace the elm.children property with a "children" property under elm.props.children that has that $$element form.

This is a symptom of a broader issue I'm experiencing opening up the Toolpad custom components API.

Your environment

npx @mui/envinfo ``` Don't forget to mention which browser you used. Output from `npx @mui/envinfo` goes here. ```

Search keywords: children

apedroferreira commented 9 months ago

~~Yeah this is a bit messy, there were always a few issues like this when supporting custom components with slots, I guess there might still be some. This might be a regression too as it probably worked before, it should be covered by tests.~~ I misunderstood the issue, nevermind.

JerryWu1234 commented 9 months ago

Is this bug only to replace elm.props.children with elm.props.$$element?”

JerryWu1234 commented 9 months ago

@Janpot There aren’t many bugs in the issue list that I can fix. So, would I like to ask for some requirements that are not included in the issue list?