[CLNP-5045] CreateChannelProvider Migration #1243
Conversation
git-babel
force-pushed
the
refactor/CLNP-5045/create-channel-migration
branch
from
37cf1d0
to
06171da
Compare
AhyoungRyu
changed the title
| userListQuery, | ||
| } = useCreateChannelContext(); | ||
| state: { | ||
| step, |
There was a problem hiding this comment.
단순 궁금증인데 step 이게 어떤걸 의미하는건가요? 코드를 보니 값이 매직넘버인것같아서 의미가 조금 명확해지도록 개선하면 좋을것 같습니다.
There was a problem hiding this comment.
채널 생성화면에서의 스텝을 말하는 것 같습니다.
(1) 생성할 채널 타입 선택
(2) 초대할 유저 선택
저도 동의합니다!
| await act(async () => { | ||
| renderComponent({ step: 1 }); | ||
| }); |
There was a problem hiding this comment.
| await act(async () => { | |
| renderComponent({ step: 1 }); | |
| }); | |
| await act(() => { | |
| renderComponent({ step: 1 }); | |
| }); |
Don't need async here since there's no usage of it.
There was a problem hiding this comment.
If async or act is deleted, the error occurs.
Warning: An update to InviteUsers inside a test was not wrapped in act(...).
When testing, code that causes React state updates should be wrapped into act(...):
The test still pass even when the act is deleted; but I added act and async to prevent the error.
| onCreateChannelClick, | ||
| onBeforeCreateChannel, | ||
| onChannelCreated, | ||
| createChannel, |
There was a problem hiding this comment.
createChannel 은 하는 역할이 action 에 들어가는게 좀 더 자연스러울 것 같아요 :)
There was a problem hiding this comment.
저도 createChannel을 어디에 넣어야하나 조금 고민을 했는데, createChannel이 CreateChannelProvider에서 관리하고 있는 어떠한 state에도 영향을 끼치지 않아서 저희 룰에 따라 state로 넣었습니다. 그럼에도 action에 들어가는 게 더 나을까요?
There was a problem hiding this comment.
당장 꺼내서 써야할 때 state vs action 둘중 어디에있을까 추측해야하는 상황이라면 저는 action 에 있을거라 예상하게 될거같아요. 그래서 좀 더 나은 사용성 측면으로는 action 에 들어있는게 자연스러울거같긴합니다!
Addresses https://sendbird.atlassian.net/browse/CLNP-5045