-
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: UpwardLinkのstyle生成をmemo化する #5244
Conversation
commit: |
951dea9
to
46fa1d6
Compare
46fa1d6
to
a3816cc
Compare
import { VariantProps, tv } from 'tailwind-variants' | ||
|
||
import { FaArrowLeftIcon } from '../Icon' | ||
import { TextLink } from '../TextLink' | ||
|
||
const upwardLink = tv({ | ||
const styleGenerator = tv({ |
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.
[要モミ]
lintで tvで生成する変数の名称を統一する
という意図が有るので /(S|s)tyleGenerator$/
にマッチする名称に変更しています。
この名称アタリ違和感ないでしょうか?
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.
個人的に違和感ないです!良いと思いました
/** `TextLink`に渡す `elementAs` をオプションで指定 */ | ||
elementAs?: ComponentProps<typeof TextLink>['elementAs'] | ||
} | ||
|
||
export const UpwardLink: React.FC<Props> = ({ indent = true, className, elementAs, ...rest }) => { | ||
const style = upwardLink({ indent, className }) | ||
const style = useMemo(() => styleGenerator({ indent, className }), [indent, className]) |
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.
smarthr-ui内で記法がブレているため、style関連の生成をuseMemoでラップするよう統一します
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.
styleGenerator()
に違和感ないのでLGTM!
import { VariantProps, tv } from 'tailwind-variants' | ||
|
||
import { FaArrowLeftIcon } from '../Icon' | ||
import { TextLink } from '../TextLink' | ||
|
||
const upwardLink = tv({ | ||
const styleGenerator = tv({ |
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
概要
変更内容
確認方法