Styling of the `Switch` component is broken when using with `ModalTrigger` component

sungik-choi commented 1 year ago

Summary

Switch 컴포넌트를 ModalTrigger 컴포넌트로 감싸서 사용했을 경우 스타일링이 깨지는 문제(background-color 적용되지 않음)가 있습니다.

Reproduction process

asChild prop이 true인 컴포넌트를 중첩해서 사용할 경우, 하위 엘리먼트에 속성이 제대로 전달되지 않음.

Version of bezier-react

1.0.0-next-v1.180

Browser

Chrome v108.0.5359.124

Operating system

[X] macOS
[ ] Windows
[ ] Linux

Additonal Information

Radix UI를 사용할 경우, 최대한 Root Component는 asChild 속성을 사용하지 않아야 asChild 를 사용한 다른 컴포넌트와 조합 시 문제 없이 사용할 수 있겠습니다.

Dialog Trigger

Switch Root

radix Dialog Trigger도 상태값을 data-state 에 담고, Switch도 마찬가지이기 때문으로 보입니다. ModalTrigger가 Switch 외부에 위치할 시 Switch의 data-state가 ModalTrigger의 것으로 오버라이드되는데, Switch의 background-color 스타일은 Switch의 data-state 값 기반 선택자로 되어있기 때문에 발생하는 문제.

sungik-choi commented 1 year ago

이외에도 radix의 Slot 을 사용하고 있는 부분들 전반적으로 체크가 필요함.

chaejunlee commented 7 months ago

radix-ui에서도 본 이슈와 비슷한 주제로 논의가 오가고 있는 것이 확인됩니다(https://github.com/radix-ui/primitives/issues/602). radix-ui에서는 primitive 간 data-state의 conflict를 어떻게 해결할지 확정하지 못한 것으로 확인됩니다 (https://github.com/radix-ui/primitives/issues/602#issuecomment-1560133746).

언급하신 경우에는 asChild의 data-state props를 최하위 component의 것으로 적용하는 것이 맞아보입니다. 하지만 radix-ui는 최상위 component의 것으로 적용하는 것으로 확인됩니다. 최하위 component의 것으로 적용하는 방식을 적용하려 한다면 radix-ui component level에서 수정이 필요해보입니다. 하지만 해당 내용으로 radix-ui에 PR을 올렸을 때 적용될 가능성은 낮아보입니다.

또 다른 방법은 background-color를 aria-checked 값 기반 선택자로 적용하는 것입니다. data-state가 아닌 다른 aria-[attribute]로 style을 적용하게 되면 이제 맞추어 확인 및 수정해야할 내용이 많아질 것 같아서 올바른 방식일지 의문이 들기는 합니다.

여러 해결 방법에 대해 각각의 장단이 있어 혹시 어떤 방법이 좋을지 의견을 여쭤보고 싶습니다!

chaejunlee commented 7 months ago

asChild가 정의된 컴포넌트 바로 아래에 display: inline 속성을 가진 태그(<a>) 또는 React.Fragment를 추가하면 어느 정도 문제가 해결되는 것 같습니다.

display: inline 속성을 가진 태그를 사용하게 되면 원래 의도와 다른 태그가 사용되어 접근성이 떨어지는 단점이 존재합니다. 하지만 functionality는 의도한 대로 작동합니다.
React.Fragment를 사용하게 되면 최하위 컴포넌트(<Switch />)의 기능이 제대로 동작하지만 그 위의 컴포넌트의 기능은 사라지는 것을 확인 했습니다.

https://github.com/radix-ui/primitives/discussions/560#discussioncomment-3477536

sungik-choi commented 6 months ago

radix-ui에서도 본 이슈와 비슷한 주제로 논의가 오가고 있는 것이 확인됩니다(radix-ui/primitives#602). radix-ui에서는 primitive 간 data-state의 conflict를 어떻게 해결할지 확정하지 못한 것으로 확인됩니다 (radix-ui/primitives#602 (comment)).

언급하신 경우에는 asChild의 data-state props를 최하위 component의 것으로 적용하는 것이 맞아보입니다. 하지만 radix-ui는 최상위 component의 것으로 적용하는 것으로 확인됩니다. 최하위 component의 것으로 적용하는 방식을 적용하려 한다면 radix-ui component level에서 수정이 필요해보입니다. 하지만 해당 내용으로 radix-ui에 PR을 올렸을 때 적용될 가능성은 낮아보입니다.

또 다른 방법은 background-color를 aria-checked 값 기반 선택자로 적용하는 것입니다. data-state가 아닌 다른 aria-[attribute]로 style을 적용하게 되면 이제 맞추어 확인 및 수정해야할 내용이 많아질 것 같아서 올바른 방식일지 의문이 들기는 합니다.

여러 해결 방법에 대해 각각의 장단이 있어 혹시 어떤 방법이 좋을지 의견을 여쭤보고 싶습니다!

@chaejunlee 자세한 리서치 감사드립니다! asChild(Slot)를 통해 병합된 상태를 data-state 라는 하나의 키를 통해 외부에 노출하게 되면 필연적으로 발생할 수 밖에 없는 문제인 거 같습니다. aria-checked 값 기반 선택자로 적용한다면 ModalTrigger 와의 충돌은 피할 수 있겠으나, data-state 와 같은 문제는 여전히 남아있습니다. aria-checked 속성을 사용하는 요소를 두 번 이상 중첩하는 경우가 있을 것 같진 않지만요.

Slot에서 prop을 merge할 때, 이벤트 핸들러 혹은 style, className과 같은 방식으로 data-state 속성을 특별 취급하여 data-state="checked closed" 처럼 resolve 해줄 수도 있겠으나 이 부분도 마찬가지로 radix-ui 라이브러리 내부 수정이 필요해보이며, 적용될 가능성이 낮아보입니다. 적어도 많은 시간이 필요할 거 같아요. data-switch-state="checked" 와 같이 data attribute에 컴포넌트별 namespace를 붙이는 방식도 마찬가지입니다.

asChild가 정의된 컴포넌트 바로 아래에 display: inline 속성을 가진 태그() 또는 React.Fragment를 추가하면 어느 정도 문제가 해결되는 것 같습니다.

이 부분은 말씀주신대로 접근성, 키보드 네비게이션 관련한 문제가 있을 수 있어 디자인 시스템의 컴포넌트 레벨에서 지원하기에는 어려워 보입니다. React.Fragment 에 대한 케이스는 정확히 어떤 케이스인지 이해를 잘 못했는데, 아마 Switch 를 React.Fragment 로 감싼 케이스로 생각됩니다. React.Fragment 는 DOM Element를 반환하지 않기 때문에 Slot의 children에 둘 경우 제대로 동작하지 않게 되는게 맞는 동작입니다.(그래서 왜 radix-ui에서 Slot의 children타입을 ReactElement가 아닌 ReactNode로 해두었는지 잘 모르겠어요)

sungik-choi commented 6 months ago

https://github.com/radix-ui/primitives/issues/602 이 이슈가 해결되기 전에는 디자인 시스템 라이브러리 차원에서는 해결되기 어려운 문제로 보입니다. 이슈가 해결되기 전까지 사용처에서 문제가 발생한다면, 중첩된 asChild(Slot) 사이에 HTML element를 하나 끼워넣는 방식으로 대응해야할 거 같아요.

channel-io / bezier-react