mui / toolpad

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

Component flicker #1050

Closed WangLarry closed 1 year ago

WangLarry commented 1 year ago

Duplicates

Latest version

Current behavior 😯

When switching page on client side, components with bounded props will flicker.

https://user-images.githubusercontent.com/5869244/192134835-f5d88595-78c2-46b2-ad0f-1df8b1aee35e.mov

Expected behavior 🤔

No response

Steps to reproduce 🕹

Steps:

1. 2. 3. 4.

Context 🔦

No response

Your environment 🌎

No response

WangLarry commented 1 year ago

fix:

function RenderedNodeContent({ node, childNodeGroups, Component }: RenderedNodeContentProps) {
  ...

  const nodeProps = (node as appDom.ElementNode).props as Record<string, any>;
  const nodeLayout = (node as appDom.ElementNode).layout as Record<string, any>;
  ...
      if (typeof hookResult[propName] === 'undefined') {
        if (
          nodeProps &&
          nodeProps[propName] &&
          (nodeProps[propName] as ConstantAttrValue<any>).type === 'const'
        ) {
          hookResult[propName] = (nodeProps[propName] as ConstantAttrValue<any>).value;
        } else if (argType) {
          hookResult[propName] = argType.defaultValue;
        }
      }
    }

 ...
      if (typeof hookResult[propName] === 'undefined') {
        if (
          nodeLayout &&
          nodeLayout[propName] &&
          (nodeLayout[propName] as ConstantAttrValue<any>).type === 'const'
        ) {
          hookResult[propName] = (nodeLayout[propName] as ConstantAttrValue<any>).value;
        } else if (argType) {
          hookResult[propName] = argType.defaultValue;
        }
     }

 ...
}

https://user-images.githubusercontent.com/5869244/192137536-c44e106f-f769-40d0-aea5-e738380e0b66.mov

WangLarry commented 1 year ago

@Janpot @apedroferreira

Janpot commented 1 year ago

@WangLarry Thank you for trying out Toolpad. Full transparency, I've been on holiday the past week and will be spending the following week in the mountains, so expect low input from my side. I will take a deeper look into these when I'm back in October.

apedroferreira commented 1 year ago

I've noticed this issue too, we'll definitely look into it and try out your suggestion - will keep you posted.

apedroferreira commented 1 year ago

fix:

function RenderedNodeContent({ node, childNodeGroups, Component }: RenderedNodeContentProps) {
  ...

  const nodeProps = (node as appDom.ElementNode).props as Record<string, any>;
  const nodeLayout = (node as appDom.ElementNode).layout as Record<string, any>;
  ...
      if (typeof hookResult[propName] === 'undefined') {
        if (
          nodeProps &&
          nodeProps[propName] &&
          (nodeProps[propName] as ConstantAttrValue<any>).type === 'const'
        ) {
          hookResult[propName] = (nodeProps[propName] as ConstantAttrValue<any>).value;
        } else if (argType) {
          hookResult[propName] = argType.defaultValue;
        }
      }
    }

 ...
      if (typeof hookResult[propName] === 'undefined') {
        if (
          nodeLayout &&
          nodeLayout[propName] &&
          (nodeLayout[propName] as ConstantAttrValue<any>).type === 'const'
        ) {
          hookResult[propName] = (nodeLayout[propName] as ConstantAttrValue<any>).value;
        } else if (argType) {
          hookResult[propName] = argType.defaultValue;
        }
     }

 ...
}

2022-09-25.17.45.21.mov

Thank you for reporting this issue and suggesting a solution, @WangLarry ! Your solution seems great to me and it fixes the issue, I've created a PR based on it at https://github.com/mui/mui-toolpad/pull/1061

Feel free to submit a PR directly next time. External contributions are always welcome!