-
Notifications
You must be signed in to change notification settings - Fork 141
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: TableコンポーネントのuseReelCells のロジックを整理する #5248
Conversation
commit: |
@@ -7,42 +7,40 @@ export const useReelCells = () => { | |||
useEffect(() => { | |||
const currentRef = tableWrapperRef.current | |||
|
|||
if (!currentRef) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
currentRefが存在しない限り、後続の処理は不要だったため、ロジックを修正しています
もともとのロジックでは各箇所にcurrentRefの存在チェックがありましたが、分散してしまい見づらかったのでここに集約させます。効率もよいです。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
早期リターンいいですね 👍
}) | ||
const stickyCells = currentRef.querySelectorAll('.fixedElement') | ||
|
||
if (!stickyCells) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
stickyCellsが存在しない場合、後続のロジックは不要だったのでifを追加しています
const shouldFix = maxScrollLeft > 0 && scrollLeft < maxScrollLeft | ||
const settableShowShadow = shouldFix | ||
? scrollLeft > 0 | ||
: maxScrollLeft !== 0 || scrollLeft !== 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
forEachの中で毎回計算する必要がなかったため、forの外で計算を済ませておきます
: maxScrollLeft !== 0 || scrollLeft !== 0 | ||
|
||
stickyCells.forEach((cell) => { | ||
cell.classList.toggle('fixed', shouldFix) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
classListに対するadd, removeはそれぞれtoggleの第2引数にtrue, falseを渡すことで表現できます。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
個人的には早期リターン推しなのでGOGOです!挙動も問題なさそうでした
@@ -7,42 +7,40 @@ export const useReelCells = () => { | |||
useEffect(() => { | |||
const currentRef = tableWrapperRef.current | |||
|
|||
if (!currentRef) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
早期リターンいいですね 👍
関連URL
概要
TableのuseReelCellsのロジックを整理し、効率化、可読性の向上を試みます
変更内容
確認方法