material-components / material-web

Material Design Web Components
https://material-web.dev
Apache License 2.0
9.31k stars 890 forks source link

Dialog animation customization is not easy to extend #5383

Open isaacwein opened 9 months ago

isaacwein commented 9 months ago

What is affected?

Component

Description

If I am using 2 dialogs on my app and I modify dialog 1 Animations with myDialog.getOpenAnimation() dialog 2 gets modified as well, is there a workaround for it

https://github.com/material-components/material-web/assets/34663972/b14d5c69-e04a-4df1-bd24-053a2b2eb09d

Reproduction

https://stackblitz.com/edit/vitejs-vite-qsxwzz?file=index.html

Workaround

i didn't find any workaround

Affected versions

i only tried on version v1.1.1

Browser/OS/Node environment

Chrome Browser 120.0.6099.217 (Official Build) (64-bit) (cohort: Stable) On my windows 11

asyncLiz commented 9 months ago

You are modifying the default implementation's core values, which is why it is unexpectedly changing everyone else's.

// You're calling the default `getOpenAnimation()` implementation, then mutating
// its results.
// myDialog2.getOpenAnimation().dialog[0][0] = [
//   { transform: 'translateX(-100%)' },
//   { transform: 'translateX(0)' },
// ];

Instead of modifying the return value of the function, you should replace the entire function and return a new object instance.

const defaultOpenAnimation = myDialog2.getOpenAnimation();
myDialog2.getOpenAnimation = () => {
  return {
    // Make a new copy of the default animation to extend from
    ...defaultOpenAnimation,
    // Add customizations
    dialog: [
      [
        [
          { transform: 'translateX(-100%)' },
          { transform: 'translateX(0)' }
        ],
        defaultOpenAnimation.dialog![0][1]
      ]
    ],
    container: [
      defaultOpenAnimation.container![0],
      [
        [
          { width: '0%' },
          { width: '100%' },
        ],
        defaultOpenAnimation.container![1][1],
      ],
    ]
  }
};

This isn't super ergonomic, so we'll keep this open to improve the devx for customizing dialog's animations.

isaacwein commented 9 months ago

thankyou it worked

https://stackblitz.com/~/github.com/isaacwein/MWC-Dialog-Bug

i did it like this

let myDialog2OpenAnimation = myDialog2.getOpenAnimation()
myDialog2OpenAnimation = deepCopy(myDialog2OpenAnimation)
myDialog2OpenAnimation.dialog![0][0] = [
  { transform: 'translateX(-100%)' },
  { transform: 'translateX(0)' },
];
myDialog2OpenAnimation.container![1][0] = [
  { width: '0%' },
  { width: '100%' },
];
myDialog2.getOpenAnimation=()=>myDialog2OpenAnimation

let myDialog2CloseAnimation = myDialog2.getCloseAnimation()
myDialog2CloseAnimation = deepCopy(myDialog2CloseAnimation)

myDialog2CloseAnimation.dialog![0][0] = [
  { transform: 'translateX(0)' },
  { transform: 'translateX(-100%)' },
];
myDialog2CloseAnimation.container![0][0] = [
  { width: '100%' },
  { width: '0%' },
];
myDialog2.getCloseAnimation=()=>myDialog2CloseAnimation
export function deepCopy(obj, hash = new WeakMap()) {
    if (obj === null || typeof obj !== 'object') {
        return obj;
    }
    if (hash.has(obj)) {
        return hash.get(obj);
    }

    // Start with a shallow copy
    let result = Array.isArray(obj) ? [...obj] : {...obj};
    hash.set(obj, result);

    // Deep copy for each property
    Object.keys(result).forEach(key => {
        result[key] = typeof result[key] === 'object' ? deepCopy(result[key], hash) : result[key];
    });

    return result;
}