Fix bug that first StackItem has marginBefore when preceding node is not JSX.Element

[yangwooseong]

Self Checklist

• I wrote a PR title in English.
• I added an appropriate label to the PR.
• I wrote a commit message in English.
• I wrote a commit message according to the Conventional Commits specification.
• I added the appropriate changeset for the changes.
• [Component] I wrote a unit test about the implementation.
• [Component] I wrote a storybook document about the implementation.
• [Component] I tested the implementation in various browsers.
  • Windows: Chrome, Edge, (Optional) Firefox
  • macOS: Chrome, Edge, Safari, (Optional) Firefox
• [New Component] I added my username to the correct directory in the CODEOWNERS file.

Related Issue

Fixes #1116

Summary

• 첫 번째 StackItem 이 원하지 않는 marginBefore 를 갖는 버그를 수정합니다.

Details

• 기존 코드에서는 valid element (=ReactElement. 참고로 string, boolean, null 등은 ReactElement 가 아닌 ReactNode 입니다) 라면 index > 0 만 검사하였기 때문에 아래와 같은 상황에서 원치 않은 marginBefore 가 들어갔습니다.

<Stack spacing={10}>
  { false && <FirstComponent /> }

  <SecondComponent /> //  marginBefore is set to 10
<Stack>

• 이제 가장 첫번째 valid element 다음에 나오는 element 에 대해서만 marginBefore 를 줍니다.
• 처음으로 했던 시도는 React.Children.toArray(children).findIndex(...) 과 같이 children 을 미리 검사하는 방법으로 첫번째 valid한 element 의 index 를 찾으려고 했으나, toArray 자체가 valid 한 element 만 filter 하기 때문에 사용하지 못했습니다. 따라서 useRef 를 사용하여 로직을 만들었습니다.

Breaking change or not (Yes/No)

• No

References

• ReactElement vs ReactNode
• Enhance Stack to give spacing properly to its StackItem children #1116

[changeset-bot]

🦋 Changeset detected

Latest commit: e67d172

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@channel.io/bezier-react	Patch
bezier-figma-plugin	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[codecov]

Codecov Report

Base: 72.75% // Head: 72.77% // Increases project coverage by +0.01% 🎉

Coverage data is based on head (e67d172) compared to base (f94c0bc).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@             Coverage Diff             @@
##           next-v1    #1120      +/-   ##
===========================================
+ Coverage    72.75%   72.77%   +0.01%     
===========================================
  Files          362      362              
  Lines         4371     4374       +3     
  Branches       852      853       +1     
===========================================
+ Hits          3180     3183       +3     
  Misses        1117     1117              
  Partials        74       74              

Impacted Files	Coverage Δ
.../bezier-react/src/components/Stack/Stack/Stack.tsx	100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[github-actions]

Chromatic Report

🚀 Congratulations! Your build was successful!

• Storybook Preview
• Chromatic Build

[sungik-choi]

Stack이 매우 많아지면 런타임 오버헤드도 무시 못할 수준일 거 같아 flex gap을 사용하는 버전도 만들어서 테스트해봐야겠네요

[yangwooseong]

Stack이 매우 많아지면 런타임 오버헤드도 무시 못할 수준일 거 같아 flex gap을 사용하는 버전도 만들어서 테스트해봐야겠네요

연산량이 그렇게 늘어날 것 같지는 않지만 flex gap 으로 하면 다소 불필요한 useRef 를 없앨 수 있겠네요! 그런데 지금은 StackItem 에만 marginBefore 를 주고 그 외의 element 에 대해서는 marginBefore 를 주지 않고 그대로 rendering 하고 있어서 이 기조를 바꿔야 flex gap 을 활용할 수 있을 것 같아요. 특정 element 사이에만 gap 을 줄 수 있는 방법이 없을 것 같아서요..!

[sungik-choi]

Stack이 매우 많아지면 런타임 오버헤드도 무시 못할 수준일 거 같아 flex gap을 사용하는 버전도 만들어서 테스트해봐야겠네요

연산량이 그렇게 늘어날 것 같지는 않지만 flex gap 으로 하면 다소 불필요한 useRef 를 없앨 수 있겠네요! 그런데 지금은 StackItem 에만 marginBefore 를 주고 그 외의 element 에 대해서는 marginBefore 를 주지 않고 그대로 rendering 하고 있어서 이 기조를 바꿔야 flex gap 을 활용할 수 있을 것 같아요. 특정 element 사이에만 gap 을 줄 수 있는 방법이 없을 것 같아서요..!

넵 이렇게 바꾼다면 StackItem 을 사용하지 않는 방향으로 기조 자체를 바꿔야할 거 같아요. 특정 element 사이에만 gap을 줘야한다면 Stack을 내부에 한번 더 감싼다던가 하는 방식으로 구현해야할 거 같습니다. 피그마 Autolayout이랑 맞춰봐야할 거 같아요