From 90ee45ae9cf6447a68540fc2414222a870afd4c4 Mon Sep 17 00:00:00 2001 From: Jeremy Alles Date: Mon, 19 Jun 2023 11:44:15 +0000 Subject: [PATCH 1/2] avoid calling getCharacterCoordinates when fullHeight is set --- src/drafts/MarkdownEditor/_MarkdownInput.tsx | 3 +-- src/drafts/hooks/useDynamicTextareaHeight.ts | 8 +++++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/drafts/MarkdownEditor/_MarkdownInput.tsx b/src/drafts/MarkdownEditor/_MarkdownInput.tsx index 6b78f2e5892..57d1e7b59ba 100644 --- a/src/drafts/MarkdownEditor/_MarkdownInput.tsx +++ b/src/drafts/MarkdownEditor/_MarkdownInput.tsx @@ -123,8 +123,7 @@ export const MarkdownInput = forwardRef return subscription?.unsubscribe }, [pasteUrlsAsPlainText]) - const dynamicHeightStyles = useDynamicTextareaHeight({maxHeightLines, minHeightLines, elementRef: ref, value}) - const heightStyles = fullHeight ? {} : dynamicHeightStyles + const heightStyles = useDynamicTextareaHeight({fullHeight, maxHeightLines, minHeightLines, elementRef: ref, value}) return ( @@ -23,6 +24,7 @@ type UseDynamicTextareaHeightSettings = { * explicitly set in CSS. */ export const useDynamicTextareaHeight = ({ + fullHeight, minHeightLines, maxHeightLines, elementRef, @@ -33,6 +35,8 @@ export const useDynamicTextareaHeight = ({ const [maxHeight, setMaxHeight] = useState(undefined) useLayoutEffect(() => { + if (fullHeight) return + const element = elementRef.current if (!element) return @@ -56,7 +60,9 @@ export const useDynamicTextareaHeight = ({ if (minHeightLines !== undefined) setMinHeight(`calc(${minHeightLines} * ${lineHeight})`) if (maxHeightLines !== undefined) setMaxHeight(`calc(${maxHeightLines} * ${lineHeight})`) // `value` is an unnecessary dependency but it enables us to recalculate as the user types - }, [minHeightLines, maxHeightLines, value, elementRef]) + }, [minHeightLines, maxHeightLines, value, elementRef, fullHeight]) + + if (fullHeight) return {} return {height, minHeight, maxHeight, boxSizing: 'content-box'} } From 718e28ef46e4f9382a1de612c2750be9df51ac41 Mon Sep 17 00:00:00 2001 From: Jeremy Alles Date: Mon, 19 Jun 2023 13:05:13 +0000 Subject: [PATCH 2/2] changeset + rename --- .changeset/honest-fishes-explain.md | 5 +++++ src/drafts/MarkdownEditor/_MarkdownInput.tsx | 9 ++++++++- src/drafts/hooks/useDynamicTextareaHeight.ts | 10 +++++----- 3 files changed, 18 insertions(+), 6 deletions(-) create mode 100644 .changeset/honest-fishes-explain.md diff --git a/.changeset/honest-fishes-explain.md b/.changeset/honest-fishes-explain.md new file mode 100644 index 00000000000..a9446ca6e4f --- /dev/null +++ b/.changeset/honest-fishes-explain.md @@ -0,0 +1,5 @@ +--- +'@primer/react': patch +--- + +improve performance of the MarkdownEditor component when fullHeight is enabled diff --git a/src/drafts/MarkdownEditor/_MarkdownInput.tsx b/src/drafts/MarkdownEditor/_MarkdownInput.tsx index 57d1e7b59ba..9d0b7e4a9e5 100644 --- a/src/drafts/MarkdownEditor/_MarkdownInput.tsx +++ b/src/drafts/MarkdownEditor/_MarkdownInput.tsx @@ -123,7 +123,14 @@ export const MarkdownInput = forwardRef return subscription?.unsubscribe }, [pasteUrlsAsPlainText]) - const heightStyles = useDynamicTextareaHeight({fullHeight, maxHeightLines, minHeightLines, elementRef: ref, value}) + const heightStyles = useDynamicTextareaHeight({ + // if fullHeight is enabled, there is no need to compute a dynamic height (for perfs reasons) + disabled: fullHeight, + maxHeightLines, + minHeightLines, + elementRef: ref, + value, + }) return ( @@ -24,7 +24,7 @@ type UseDynamicTextareaHeightSettings = { * explicitly set in CSS. */ export const useDynamicTextareaHeight = ({ - fullHeight, + disabled, minHeightLines, maxHeightLines, elementRef, @@ -35,7 +35,7 @@ export const useDynamicTextareaHeight = ({ const [maxHeight, setMaxHeight] = useState(undefined) useLayoutEffect(() => { - if (fullHeight) return + if (disabled) return const element = elementRef.current if (!element) return @@ -60,9 +60,9 @@ export const useDynamicTextareaHeight = ({ if (minHeightLines !== undefined) setMinHeight(`calc(${minHeightLines} * ${lineHeight})`) if (maxHeightLines !== undefined) setMaxHeight(`calc(${maxHeightLines} * ${lineHeight})`) // `value` is an unnecessary dependency but it enables us to recalculate as the user types - }, [minHeightLines, maxHeightLines, value, elementRef, fullHeight]) + }, [minHeightLines, maxHeightLines, value, elementRef, disabled]) - if (fullHeight) return {} + if (disabled) return {} return {height, minHeight, maxHeight, boxSizing: 'content-box'} }