Skip to content
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: テキスト系のシンプルなコンポーネントをmemo化する #5406

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

AtsushiM
Copy link
Member

関連URL

概要

  • 再レンダリングする必要性が薄い場合が多く、かつ関数・オブジェクトなどのmemoが聞きにくい値も渡される可能性が引くコンポーネントをmemo化したい

変更内容

  • memoを適用
    • その際、genericsが引き渡されるように調整

確認方法

@yagimushi yagimushi force-pushed the fix-memoized-simple-component branch from bcbc89a to 254cc97 Compare February 20, 2025 23:32
@yagimushi yagimushi force-pushed the fix-memoized-simple-component branch from 254cc97 to 8ef3027 Compare February 20, 2025 23:35
Copy link

pkg-pr-new bot commented Feb 20, 2025

Open in Stackblitz

npm i https://pkg.pr.new/kufu/smarthr-ui@5406

commit: 2b94b5a

}

export const Text = memo(ActualText) as typeof ActualText
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


return (
<div className={style}>
<TextLink {...rest} elementAs={elementAs} prefix={<FaArrowLeftIcon />} />
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

elementAsが消えているように見えますが、https://github.com/kufu/smarthr-ui/pull/5406/files#diff-5f096a050e1946adb3b1a45c9737159ebceea44f39788f072ea5b19eee4385baR23 でrestに含めるようにしているため、指定としては同じ状態になっています

import { tv } from 'tailwind-variants'

export const visuallyHiddenText = tv({
export const visuallyHiddenTextClassNameGenerator = tv({
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

visuallyHiddenText ではコンポーネントと区別がつきにくいため、明確にしました。
長くてちょっと...とも思いますが基本的にsmarthr-ui内に隠蔽される概念のため、空目・typoよりはマシかと...

@AtsushiM AtsushiM marked this pull request as ready for review February 24, 2025 23:04
@AtsushiM AtsushiM requested a review from a team as a code owner February 24, 2025 23:04
@AtsushiM AtsushiM requested review from moshisora and nabeliwo and removed request for a team February 24, 2025 23:04
Copy link
Contributor

@oti oti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants