Fix/vertical spacing #160

meganrm · 2025-11-15T00:40:06Z

Problem

the expandable text pushed the re-run button down
Also, we weren't resizing the app if someone resized the window
EDIT after comment:
we want the tab headers and description to be fixed at the top

Solution

used a hook and some state management to re-calc heights after collapse/expand text

Type of change

Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)

Steps to Verify:

bun dev
expand/collapse text
switch recipes
resize window

Copilot

Pull Request Overview

This PR fixes vertical spacing issues caused by expandable text affecting layout and adds window resize handling. The expandable text component was previously pushing the re-run button down, and the app wasn't responding to window resizes.

Key Changes:

Introduced dynamic height calculation using a custom hook that responds to window resize events
Added state management to track description height changes when text expands/collapses
Refactored component to use calculated heights instead of CSS-based height calculations

Reviewed Changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/hooks/useSiderHeight.tsx	New hook that calculates and updates sidebar height on window resize
src/constants.ts	New constants file defining component height values
src/components/ExpandableText/index.tsx	Refactored component with height tracking callback and expand/collapse state
src/components/PackingInput/index.tsx	Integrated height calculations and passes computed heights to child components
src/components/RecipeForm/index.tsx	Updated to accept and apply dynamic height prop
src/components/RecipeForm/style.css	Removed static CSS height calculation
src/components/PackingInput/style.css	Removed unused recipe-content height CSS
src/App.tsx	Applied dynamic sidebar height from hook
src/App.css	Removed static CSS height calculation for sidebar
src/components/ExpandableDescription/index.tsx	Deleted old component file (moved to ExpandableText)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/components/PackingInput/index.tsx

src/components/ExpandableText/index.tsx

github-actions · 2025-11-15T00:40:56Z

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	18.93%	368 / 1944
🔵	Statements	18.93%	368 / 1944
🔵	Functions	38.09%	24 / 63
🔵	Branches	69.23%	81 / 117

File Coverage

File	Stmts	% Branch	% Funcs	% Lines	Uncovered Lines
Changed Files
src/App.tsx	0%	0%	0%	0%	1-7, 16-19, 21, 23-24, 26-27, 29-35, 37, 39-41, 43-49, 51-65, 67-110, 112-120, 122-157, 159-163, 165-169, 171-190, 192, 194
src/constants.ts	0%	0%	0%	0%	1-4
src/components/ExpandableText/index.tsx	0%	0%	0%	0%	1-3, 7, 9-10, 17-20, 23-26, 29-31, 33-35, 37-55, 57-58, 60-61
src/components/JSONViewer/index.tsx	0%	0%	0%	0%	1-2, 8-9, 18-19, 21-23, 26, 28, 30-42, 45-55, 57-61, 65-68, 70-93, 95, 97
src/components/PackingInput/index.tsx	0%	0%	0%	0%	1-2, 12-18, 24, 34-38, 40-44, 46-51, 53-57, 59-62, 65-67, 69-71, 73, 76-78, 80-90, 92-93, 95-102, 104-109, 111-118, 120-123, 125, 127, 129
src/components/RecipeForm/index.tsx	0%	0%	0%	0%	1-3, 9, 16-20, 22-42, 44-47, 50-56, 58-60, 62, 64-65
src/hooks/useSiderHeight.tsx	0%	0%	0%	0%	1-3, 5-8, 10-18, 20-21

Generated in workflow #193

github-actions · 2025-11-15T00:41:05Z

PR Preview Action v1.6.2
Preview removed because the pull request was closed.
2025-11-19 18:17 UTC

…ellpack-client into fix/vertical-spacing

meganrm · 2025-11-15T00:44:43Z

src/components/ExpandableText/index.tsx

@@ -0,0 +1,57 @@
+import { Typography } from "antd";


I renamed this folder so the component name matched the file name

meganrm · 2025-11-15T00:45:17Z

src/components/ExpandableText/index.tsx

+    const [isExpanded, setIsExpanded] = useState<boolean>(false);
+    const ref = useRef<HTMLParagraphElement>(null);
+
+    useEffect(() => {
+        setCurrentHeight(
+            (ref.current?.clientHeight || DEFAULT_DESCRIPTION_HEIGHT) +
+                TEXT_BOTTOM_MARGIN
+        );
+    }, [isExpanded, setCurrentHeight]);


these are the main changes, I needed to have a way of tracking after the re-render was complete to then get the new height of the element

Copilot

Pull Request Overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/components/PackingInput/index.tsx

Co-authored-by: Copilot <[email protected]>

…ellpack-client into fix/vertical-spacing

interim17

Looks good on my machine

ascibisz · 2025-11-17T14:52:15Z

Thao made this request on slack (it's also in the design doc)

I’ll suggest a ticket for later to make only the editable fields area scrollable:

Recipe selection, description, and the tabs (Editable fields/Full recipe) are fixed on top

Re-run button is fixed on bottom

The fields in the middle are scrollable if their height exceeds the available space

Does it make sense to do this as part of this PR, or as a follow up ticket?

meganrm · 2025-11-17T19:07:02Z

Thao made this request on slack (it's also in the design doc)

I’ll suggest a ticket for later to make only the editable fields area scrollable:

Recipe selection, description, and the tabs (Editable fields/Full recipe) are fixed on top

Re-run button is fixed on bottom

The fields in the middle are scrollable if their height exceeds the available space

Does it make sense to do this as part of this PR, or as a follow up ticket?

ah thanks! yeah, I think it makes sense to re-work this pr with that in mind. I'll convert this to a draft

Copilot

Pull Request Overview

Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-18T01:18:28Z

src/hooks/useSiderHeight.tsx

+    useEffect(() => {
+        function handleResize() {
+            setSiderHeight(window.innerHeight - HEADER_HEIGHT - FOOTER_HEIGHT);
+        }
+        window.addEventListener("resize", handleResize);
+        return () => {
+            window.removeEventListener("resize", handleResize);
+        };
+    }, []);


The height calculation logic is duplicated in the initial state (line 7) and the resize handler (line 12). Extract this into a helper function to avoid duplication and ensure consistency.

src/components/ExpandableText/index.tsx

meganrm · 2025-11-18T21:21:32Z

Thao made this request on slack (it's also in the design doc)

I’ll suggest a ticket for later to make only the editable fields area scrollable:

Recipe selection, description, and the tabs (Editable fields/Full recipe) are fixed on top

Re-run button is fixed on bottom

The fields in the middle are scrollable if their height exceeds the available space

Does it make sense to do this as part of this PR, or as a follow up ticket?

ah thanks! yeah, I think it makes sense to re-work this pr with that in mind. I'll convert this to a draft

it's now updated

Copilot

Pull Request Overview

Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-19T01:50:03Z

src/components/ExpandableText/index.tsx

+            (ref.current?.clientHeight || DEFAULT_DESCRIPTION_HEIGHT) +
+                TEXT_BOTTOM_MARGIN
+        );
+    }, [isExpanded, setCurrentHeight]);


The useLayoutEffect hook is missing text in its dependency array. When the text changes, useEffect resets isExpanded to false, but the height may not be recalculated if setCurrentHeight is a stable reference. This can lead to stale height values. Add text to the dependency array of useLayoutEffect.

Suggested change

}, [isExpanded, setCurrentHeight]);

}, [isExpanded, setCurrentHeight, text]);

Copilot · 2025-11-19T01:50:03Z

src/components/PackingInput/index.tsx

+    const getAvailableHeight = useCallback(() => {
+        return siderHeight - descriptionHeight - SELECT_HEIGHT;
+    }, [siderHeight, descriptionHeight]);
+
+    useEffect(() => {
+        const newAvailableHeight = getAvailableHeight();
+        setAvailableRecipeHeight(newAvailableHeight);
+    }, [getAvailableHeight]);


The getAvailableHeight callback and the effect that uses it are redundant. The effect can directly calculate and set the available height without the intermediate callback, simplifying the code and removing unnecessary memoization.

Suggested change

const getAvailableHeight = useCallback(() => {

return siderHeight - descriptionHeight - SELECT_HEIGHT;

}, [siderHeight, descriptionHeight]);

useEffect(() => {

const newAvailableHeight = getAvailableHeight();

setAvailableRecipeHeight(newAvailableHeight);

}, [getAvailableHeight]);

// Update availableRecipeHeight when siderHeight or descriptionHeight changes

useEffect(() => {

const newAvailableHeight = siderHeight - descriptionHeight - SELECT_HEIGHT;

setAvailableRecipeHeight(newAvailableHeight);

}, [siderHeight, descriptionHeight]);

meganrm added 3 commits November 14, 2025 15:48

adjusting height correctly

cf7a7b7

button stays in the same spot

459b2da

remove commented out code

a938ba5

meganrm requested review from Copilot and interim17 November 15, 2025 00:40

Merge branch 'main' into fix/vertical-spacing

203214b

Copilot AI reviewed Nov 15, 2025

View reviewed changes

src/components/PackingInput/index.tsx Outdated Show resolved Hide resolved

src/components/ExpandableText/index.tsx Outdated Show resolved Hide resolved

meganrm requested review from ascibisz and rugeli November 15, 2025 00:40

meganrm added 3 commits November 14, 2025 16:41

remove ref

8310931

Merge branch 'fix/vertical-spacing' of https://github.com/AllenCell/c…

c1ca39a

…ellpack-client into fix/vertical-spacing

fix bug

628c093

meganrm commented Nov 15, 2025

View reviewed changes

meganrm requested a review from Copilot November 15, 2025 00:46

Copilot AI reviewed Nov 15, 2025

View reviewed changes

src/components/PackingInput/index.tsx Outdated Show resolved Hide resolved

src/components/PackingInput/index.tsx Outdated Show resolved Hide resolved

meganrm and others added 4 commits November 14, 2025 16:47

Update src/components/PackingInput/index.tsx

92d558d

Co-authored-by: Copilot <[email protected]>

Update src/components/PackingInput/index.tsx

524fc37

Co-authored-by: Copilot <[email protected]>

re-order imports

19abf91

Merge branch 'fix/vertical-spacing' of https://github.com/AllenCell/c…

9b71b89

…ellpack-client into fix/vertical-spacing

interim17 approved these changes Nov 15, 2025

View reviewed changes

meganrm marked this pull request as draft November 17, 2025 19:24

meganrm added 3 commits November 17, 2025 14:11

scroll within recipe container

9335fc8

maintain height

bcf948c

remove sider height

58b0902

meganrm marked this pull request as ready for review November 18, 2025 01:17

meganrm requested a review from Copilot November 18, 2025 01:17

Copilot AI reviewed Nov 18, 2025

View reviewed changes

meganrm requested a review from interim17 November 18, 2025 01:18

uselayouteffect

cbcc139

ascibisz approved these changes Nov 18, 2025

View reviewed changes

meganrm requested a review from Copilot November 19, 2025 01:49

Copilot AI reviewed Nov 19, 2025

View reviewed changes

remove function

b6378ba

interim17 approved these changes Nov 19, 2025

View reviewed changes

meganrm merged commit ceb85ce into main Nov 19, 2025
2 checks passed

meganrm deleted the fix/vertical-spacing branch November 19, 2025 18:16

	}, [isExpanded, setCurrentHeight]);
	}, [isExpanded, setCurrentHeight, text]);

Fix/vertical spacing #160

Fix/vertical spacing #160

Uh oh!

Conversation

meganrm commented Nov 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

Type of change

Steps to Verify:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Nov 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report

Uh oh!

github-actions bot commented Nov 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

meganrm Nov 15, 2025

Choose a reason for hiding this comment

Uh oh!

meganrm Nov 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

interim17 left a comment

Choose a reason for hiding this comment

Uh oh!

ascibisz commented Nov 17, 2025

Uh oh!

meganrm commented Nov 17, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

meganrm commented Nov 18, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

meganrm commented Nov 15, 2025 •

edited

Loading

github-actions bot commented Nov 15, 2025 •

edited

Loading

github-actions bot commented Nov 15, 2025 •

edited

Loading