fix: mobile editor page title display#10712
Conversation
satof3
changed the title
| window.addEventListener('resize', calcMaxWidth); | ||
| return () => { | ||
| window.removeEventListener('resize', calcMaxWidth); | ||
| }; |
There was a problem hiding this comment.
この addEventListener はパフォーマンス悪化の原因になるので消してください
useDeviceLargerThanSm を使えば re-rendering されるはず
There was a problem hiding this comment.
useDeviceLagerThanSm を作り適用させました
| calcMaxWidth(); | ||
| }, [calcMaxWidth]); | ||
| // Update the style directly if needed, or use state management | ||
| // setMaxWidth(maxWidth); |
There was a problem hiding this comment.
これでは calcMaxWidth が役割を失っている
元の useEffect による calcMaxWidth 実行方式から変えた理由はなに？
There was a problem hiding this comment.
useEffect に戻しました。
calcMaxWidth は PageHeader の x 座標はリネーム・移動後も変化しないため、 onRenameTerminated / onMoveTerminatedに渡す必要がなく、外部から呼ぶ機会がなくなったため削除し、 useEffect 内に直接計算を書く形に整理しました。
| maxWidth={maxWidth} | ||
| onRenameTerminated={calcMaxWidth} | ||
| /> | ||
| <PagePathHeader currentPage={currentPage} maxWidth={maxWidth} /> |
There was a problem hiding this comment.
デグレの可能性を排除できないので、onRenameTerminated と onMoveTerminated による再計算は残してほしい
なぜか？
rename, move によって PageControls.x 位置が変わらないという保証がない
別コンポーネントの挙動だから、PageHeader が変わる or 変わらないを知っているはずがない
別の問題 (かもしれないもの)
サイドバー開閉で pageHeaderX が変わった場合に再計算されないかもしれない
PagePathNavSticky が sidebarWidth を依存配列に入れているように、このコンポーネントでもサイドバー開閉時の pageHeaderX 変化への対応が必要かもしれない
There was a problem hiding this comment.
確認したところ現状では実質no-opのようですが将来を含めた保証という観点からuseCallbackの削除を元通りにしました
別の問題 (かもしれないもの)
についてはEditorではサイドバーの開閉によってwidth は再計算されないはずなので対応は不要だと考えています
|
@mergify queue |
Merge Queue Status
This pull request spent 13 minutes 34 seconds in the queue, including 13 minutes 20 seconds running CI. Required conditions to merge
|
2 participants
Task
https://redmine.weseek.co.jp/issues/176753
Summary