fix: Textareaの文字数カウンタをonChangeで処理するように修正 #4916

atzzCokeK · 2024-09-12T14:24:48Z

概要

keyUpでカウントしているためコピペ時に文字数カウンタが更新されない状態になっていた。
そのため、onChangeで処理を行うことにより、コピペしたときでも文字数がカウントされるように修正する。

またonChangeだと、レンダリング等のパフォーマンスに影響がある可能性があるためDebounce処理を導入しています。

変更内容

入力文字数処理をonKeyUpからonChangeに変更する
debounce処理を導入しパフォーマンスを向上させる
テストの一部修正
debounce関数の実装

※1. ローカルで触ってみて違和感なかったですが、もし違和感や懸念がありそうであれば200ms を変更するか、スロットリングに変更します。
※2. 200msの根拠→ユーザにとって違和感がない値で設定したいため、ここで言われている200~500msの中でいうと200msで設定することにしました。
参考：How fast should your UI animations be? | Val Head

動作確認

Before

2024-09-12.23.53.52.mov

After

2024-09-12.23.57.19.mov

確認方法

Storybookでご確認ください 🙇

atzzCokeK · 2024-09-12T15:00:57Z

packages/smarthr-ui/src/components/Textarea/Textarea.stories.tsx

+  const [value1, setValue1] = useState('message👌')
+  const [value2, setValue2] = useState('message👌')
+  const [value3, setValue3] = useState('message👌')


同一のstateが利用されていたため、1つのコンポーネントで変更したものが他に影響を与えていたため修正しました。

pkg-pr-new · 2024-09-12T15:35:26Z

Open in Stackblitz

pnpm add https://pkg.pr.new/kufu/smarthr-ui@4916

commit: c4f86c3

atzzCokeK · 2024-09-12T15:41:47Z

packages/smarthr-ui/src/libs/debounce.ts

+  func: T,
+  wait: number,
+): ((...args: Parameters<T>) => void) => {
+  let timeoutId: NodeJS.Timeout | string | number | null = null


clearTimeoutの引数の型定義を参考にした

/** * Cancels a `Timeout` object created by `setTimeout()`. * @since v0.0.1 * @param timeout A `Timeout` object as returned by {@link setTimeout} or the `primitive` of the `Timeout` object as a string or a number. */

※ブラウザ環境での実行時にはsetTimeoutはnumberを返す

ReturnType<typeof setTimeout> でブラウザ環境もNode環境もよしなに型判定してくれるかも？

atzzCokeK · 2024-09-12T15:46:25Z

packages/smarthr-ui/src/libs/debounce.ts

パッケージを導入せず独自実装した理由は、要件がシンプルだったため、わざわざ外部ライブラリに依存する必要がなく、独自実装で十分そうと判断したためです。

またlodash.debounce を用いた実装も、lodash.mergeやlodash.range同様に導入してみました。
ですが、ビルド時に以下のようなエラーが発生し、解決に時間がかかりそうだったので中断しました。

src/libs/lodash.ts:7:14 - error TS2742: The inferred type of 'debounce' cannot be named without a reference to '.pnpm/@[email protected]/node_modules/@types/lodash'. This is likely not portable. A type annotation is necessary. 7 export const debounce = _debounce

全然スコープ外の話をするんですけど、この debounce 関数をプロダクトでも使いたい…。
任意のライブラリ導入したり、これみたいに自前で実装すれば良い話ではあるんですが、せっかくなら社内全体で汎用的に使える状態になってると嬉しいなって思いました。tamatebako 案件かも。

@s-sasaki-0529 実際にtamatebakoに入ったら使われそうですか？:eyes:
もしニーズ有りそうであればあればtamatebakoに移そうかな〜と思います！tokky-sanと話して進めていくべきなんですかね？tamatebako全然見てなかったので、どなたかの許可とかいるのか気になっています。 @Tokky0425

OSS なので、PR 投げましょう!

ぶん投げまーす🥋🔥

oti

実装よさそうです！　一点だけ変数名でコメントしました:bow:

oti · 2024-09-13T01:36:29Z

packages/smarthr-ui/src/components/Textarea/Textarea.tsx

    )
+
+    const handleOnChange = useCallback(


Suggested change

const handleOnChange = useCallback(

const handleOnChange = useCallback(

nits: Changeイベントのハンドラなので、変数は handleChange がよさそうです！

@oti ありがとうございます！修正します！

@oti requestChangeでレビューいただいていたので、aprvいただけますと 🙇

hiroki0525 · 2024-09-13T01:53:51Z

packages/smarthr-ui/src/components/Textarea/Textarea.tsx

+    // eslint-disable-next-line react-hooks/exhaustive-deps
+    const debouncedUpdateCount = useCallback(
+      debounce((value) => {
+        setCount(getStringLength(value))


優先度の低いstate更新だと思うので startTransition を使っても良さそうです

https://ja.react.dev/reference/react/startTransition

@hiroki0525 これまで実際に使ったことなかったんですが、入れてみました🙇！勉強になりました、ありがとうございます！

hiroki0525 · 2024-09-13T01:57:41Z

packages/smarthr-ui/src/libs/debounce.ts

+    if (timeoutId !== null) {
+      clearTimeout(timeoutId)
+    }
+    timeoutId = setTimeout(() => {


setTimeout 使うなら requestIdleCallback や schedular.postTask あたりを使ったほうがよりベターですが、対応していないブラウザの考慮が必要＆そこまでパフォーマンスチューニングしなくても良いと思うので setTimeout で良さそう！

@hiroki0525 ガーサス、詳しい...!!! nukosuke-sanの自作ライブラリいずれ入れましょう！ 🔥

s-sasaki-0529 · 2024-09-13T02:52:13Z

packages/smarthr-ui/src/components/Textarea/Textarea.tsx

+      debounce((value) => {
+        startTransition(() => {
+          setCount(getStringLength(value))
+        })


startTransition についてあまり知らないので教えてほしいです！

これは関数内での状態更新の優先度を下げることで UI のブロッキングを解消してひっそりと更新する用途という理解なんですが、この機能があれば debouce せずとも元々の課題だったUIのカクつきを回避しながら onChange で直越更新できるってこと無いですかね…？

これは関数内での状態更新の優先度を下げることで UI のブロッキングを解消してひっそりと更新する用途という理解

私も同じような理解をしています（存在は知ってたけど使ったことなかった）

debouce せずとも元々の課題だったUIのカクつきを回避しながら onChange で直越更新できるってこと無いですかね…？

可能性としてはありそうですが、setCountの実行回数自体は変わらないはずなのでやはりパフォーマンス懸念は残ってしまうのかも？とか想像しましたが、自信はないです...

@hiroki0525 nukosuke-sanお詳しそうなので、このあたりご意見や知見ありますか？ 🙇

これは関数内での状態更新の優先度を下げることで UI のブロッキングを解消してひっそりと更新する用途という理解

自分も同じ理解です 😇

この機能があれば debouce せずとも元々の課題だったUIのカクつきを回避しながら onChange で直越更新できるってこと無いですかね…？

もしかしたら startTransition だけでも十分かもしれませんね...

お二人ともありがとうございます〜！一旦はこちらの実装でいこうと思うんですが、リファクタ可能かもということで認識しておきます...🙇 !
startTransition解像度高めたい

s-sasaki-0529 · 2024-09-13T02:56:02Z

packages/smarthr-ui/src/components/Textarea/Textarea.tsx

+
+    const handleChange = useCallback(
+      (event: React.ChangeEvent<HTMLTextAreaElement>) => {
+        onChange && onChange(event)


ここ onChange 呼び出し結果の真偽値まで見てるのどういう意図あるんですっけ…？

@s-sasaki-0529
TextareaのonChangeの定義が以下のようになっているので、onChangeがundefinedの可能性があるからこのような実装にしています〜 🙇

React.ChangeEventHandler | undefined

s-sasaki-0529

👍 さっそく pgk.pr.new 使ってプロダクト側で動作を確認してみましたが、動作良い感じに改善されてそうでした！マウスでコピペしても反映されることも確認OKです！

hiroki0525

👍

oti

LGTM!

atzzCokeK self-assigned this Sep 12, 2024

fix: Textareaの文字数カウンタをonChangeで処理するように修正

4f29ad8

atzzCokeK force-pushed the fix/SHRUI-1049 branch from 8df46f2 to 4f29ad8 Compare September 12, 2024 14:34

chore: テスト修正

c777744

atzzCokeK commented Sep 12, 2024

View reviewed changes

chore: debounce処理を自前で実装

91e1c01

fix: 型を修正

6966078

atzzCokeK commented Sep 12, 2024

View reviewed changes

atzzCokeK marked this pull request as ready for review September 13, 2024 00:59

atzzCokeK requested a review from a team as a code owner September 13, 2024 00:59

atzzCokeK requested review from Qs-F and s-sasaki-0529 and removed request for a team September 13, 2024 00:59

oti requested changes Sep 13, 2024

View reviewed changes

hiroki0525 reviewed Sep 13, 2024

View reviewed changes

chore: startTransitionの導入

c4f86c3

atzzCokeK requested review from hiroki0525 and oti September 13, 2024 02:17

s-sasaki-0529 reviewed Sep 13, 2024

View reviewed changes

s-sasaki-0529 approved these changes Sep 13, 2024

View reviewed changes

hiroki0525 approved these changes Sep 13, 2024

View reviewed changes

atzzCokeK enabled auto-merge (squash) September 13, 2024 07:11

oti approved these changes Sep 13, 2024

View reviewed changes

atzzCokeK merged commit a054e07 into master Sep 13, 2024
10 checks passed

atzzCokeK deleted the fix/SHRUI-1049 branch September 13, 2024 07:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Textareaの文字数カウンタをonChangeで処理するように修正 #4916

fix: Textareaの文字数カウンタをonChangeで処理するように修正 #4916

atzzCokeK commented Sep 12, 2024 •

edited

Loading

atzzCokeK Sep 12, 2024

pkg-pr-new bot commented Sep 12, 2024 •

edited

Loading

atzzCokeK Sep 12, 2024

atzzCokeK Sep 12, 2024

hiroki0525 Sep 13, 2024

atzzCokeK Sep 12, 2024

s-sasaki-0529 Sep 13, 2024

atzzCokeK Sep 13, 2024

uknmr Sep 13, 2024

atzzCokeK Sep 13, 2024

oti left a comment

oti Sep 13, 2024

atzzCokeK Sep 13, 2024

atzzCokeK Sep 13, 2024

hiroki0525 Sep 13, 2024

atzzCokeK Sep 13, 2024

hiroki0525 Sep 13, 2024 •

edited

Loading

atzzCokeK Sep 13, 2024

s-sasaki-0529 Sep 13, 2024

atzzCokeK Sep 13, 2024

hiroki0525 Sep 13, 2024

atzzCokeK Sep 13, 2024

s-sasaki-0529 Sep 13, 2024

atzzCokeK Sep 13, 2024 •

edited

Loading

s-sasaki-0529 left a comment

hiroki0525 left a comment

oti left a comment

	const handleOnChange = useCallback(
	const handleOnChange = useCallback(

fix: Textareaの文字数カウンタをonChangeで処理するように修正 #4916

fix: Textareaの文字数カウンタをonChangeで処理するように修正 #4916

Conversation

atzzCokeK commented Sep 12, 2024 • edited Loading

関連URL

概要

変更内容

動作確認

Before

After

確認方法

Choose a reason for hiding this comment

pkg-pr-new bot commented Sep 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oti left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hiroki0525 Sep 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

atzzCokeK Sep 13, 2024 • edited Loading

Choose a reason for hiding this comment

s-sasaki-0529 left a comment

Choose a reason for hiding this comment

hiroki0525 left a comment

Choose a reason for hiding this comment

oti left a comment

Choose a reason for hiding this comment

atzzCokeK commented Sep 12, 2024 •

edited

Loading

pkg-pr-new bot commented Sep 12, 2024 •

edited

Loading

hiroki0525 Sep 13, 2024 •

edited

Loading

atzzCokeK Sep 13, 2024 •

edited

Loading