chakra-ui icon indicating copy to clipboard operation
chakra-ui copied to clipboard

DrawerContent does not properly forward needed props for controlling the animation

Open mix3d opened this issue 3 years ago • 4 comments

Description

When I add a transition prop to the <DrawerContent>, I expect the DrawerContent to pass that through to the Slider / motion.div, but it seems like it gets swallowed at the StyledComponent level, possibly due to naming conflicts.

Link to Reproduction

https://codesandbox.io/s/chakra-ui-javascript-lzzg9?file=/src/index.js

Steps to reproduce

  1. Recreate the sandbox link code in a place where you can use React Devtools
  2. Open the Drawer by clicking the button
  3. Inspect the DrawerContent and subsequent Slide component.
  4. Observe that the transitionEnd prop is passed through to the MotionComponent (motion.div), but the transition prop is swallowed at the parent StyledSlide factory component.

Chakra UI Version

1.8.7

Browser

Chrome v100

Operating System

  • [X] macOS
  • [X] Windows
  • [ ] Linux

Additional Information

I would assume that the styled component is swallowing the transition prop, expecting it to be a CSS value, and not an object prop expecting to be passed down to children. The way the DrawerContent component is written, we cannot directly pass in a variants prop like the popoverContent and ModalContent can.

The Slide component's motion.div will never allow for overwriting the transition animation if it cannot receive it. I can, of course, pass through a whole new variants prop, but all of this is undocumented functionality that I discovered through hours of digging through the source code, just trying to change the top height of where the drawer renders.

I think if this actually worked, and there was adequate documentation to show devs how to use it, the way you've setup the animation passthrough to the variant via props is a neat way to go. But the naming could use some help AND the fact that it doesn't seem to work at all is unfortunate.

Parent Slide styled component: image

Child Slide root component: image

mix3d avatar Apr 15 '22 23:04 mix3d

Tangentially related, it seems like the computedStyle of the Slide component is also not getting the correct styles passed through from the DrawerContent component. The Styled Component wrapper is creating a CSS class for the Slide, but the Slide component is still applying style attributes that override them with specificity, and no clear way to pass through style props otherwise.

// Slide Component
    const computedStyle: MotionStyle = Object.assign(
      { position: "fixed" },
      transitionStyles.position,
      style,
    )
    ...
   <motion.div
        {...rest}
        ref={ref}
        initial="exit"
        className={cx("chakra-slide", className)}
        animate={animate}
        exit="exit"
        custom={custom}
        variants={variants as TVariants}
        style={computedStyle} // This becomes a style attribute
    />
    
    // Drawer
    const dialogStyles: SystemStyleObject = {
      display: "flex",
      flexDirection: "column",
      position: "relative", 
      width: "100%",
      outline: 0,
      ...styles.dialog,
    }
    ...
    <StyledSlide
        direction={placement}
        in={isOpen}
        className={_className}
        {...dialogProps}
        __css={dialogStyles} // This becomes a CSS class.
      >
        {children}
    </StyledSlide>

mix3d avatar Apr 16 '22 00:04 mix3d

@mix3d for what it's worth I'm running into the exact same problem(s) that you are and am invested in some solution here. Maybe worth saying - I followed your link to a reproduction and it doesn't include a reproduction, it seemed to be some sort of Chakra codesandbox template.

To tack on to the problem(s) here, in my case I wanted the Drawer to be position: sticky instead of position: fixed. I managed to pull that off with the following:

image

(potentially worth showing just because I have to use internals and augment the ModalContext to get anywhere near what I'm going for, which I'm sure isn't intended)

The result of this is that the animation looks very strange so I wanted to edit it, and now I'm here.

joshacheson avatar May 15 '22 14:05 joshacheson

I did a bit more research into this and discovered that the chakra factory (to which Slide is passed) accepts an optional second argument options which takes a shouldForwardProp value. Works like this

import { Slide } from '@chakra/transitions';
import { shouldForwardProp as defaultShouldForwardProp, chakra } from '@chakra-ui/system';

const propsToForward = ['transition', 'transitionEnd'];

const StyledSlide = chakra(Slide, {
  shouldForwardProp: (prop: string) => {
    return propsToForward.includes(prop) ? true : defaultShouldForwardProp(prop);
  }
});

I think if this change is implemented, that's enough to solve this issue for both of our cases (that is assuming there's a set of props the chakra team is up for allowing specifically for this Slide component). In my own case, for now, I've just copy pasted DrawerContent into my codebase and this change works great.

joshacheson avatar May 18 '22 20:05 joshacheson

I've just copy pasted DrawerContent into my codebase and this change works great

But this is exactly why I raised the bug in the first place; if I am running into this problem, others will too. "fix the source in your own repo" is a last resort.

mix3d avatar Jun 29 '22 22:06 mix3d