fix: soup crashing safari at certain widths

[peterchinman]

No description provided.

[coderabbitai]

📝 Walkthrough

Summary by CodeRabbit

• Bug Fixes
  • Improved split-layout header collapse/expand functionality with enhanced overflow detection accuracy and reduced unnecessary state toggling for smoother interactions.

Walkthrough

Optimized header collapse behavior in the split-layout component by introducing debounced evaluation scheduling via requestAnimationFrame, improving overflow detection with content-width comparison, and adding hysteresis logic to reduce state toggling. Enhanced cleanup procedures for observer management.

Changes

Cohort / File(s)	Summary
Header Collapser Optimization js/app/packages/app/component/split-layout/utils/createHeaderCollapser.ts	Added scheduleEvaluate debouncer using requestAnimationFrame to coalesce repeated triggers. Replaced scrollWidth/offsetWidth overflow detection with content-width comparison using getContentWidth() and OVERFLOW_EPSILON_PX tolerance. Introduced UNCOLLAPSE_HYSTERESIS_PX to reduce state toggling. Enhanced cleanup to cancel pending animation-frame evaluation and disconnect observer.

🚥 Pre-merge checks | ✅ 1 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	❓ Inconclusive	No pull request description was provided by the author, making it impossible to assess relevance to the changeset.	Add a description explaining the Safari crash issue, the root cause, and how the debouncing and hysteresis changes resolve it.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The pull request title follows conventional commits format (fix:) and is 43 characters, well under the 72-character limit. It clearly describes the fix for Safari crashing at certain widths.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

Preview: https://peter-safari-crashing-6xbk98.preview.macro.com/app (eadf060)

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@js/app/packages/app/component/split-layout/utils/createHeaderCollapser.ts`:
- Around line 65-68: In createHeaderCollapser the ResizeObserver instance
(observer) is only attached once to the initial header element and never
re-attached if getContainer() later returns a new node; update the logic so
whenever headerLeft is resolved you call observer.observe(headerLeft) (and if
the resolved element differs from the previously-observed node, call
observer.unobserve(prev) or observer.disconnect() first) and ensure you store
the currentlyObserved element so future container swaps rebind the observer;
keep using scheduleEvaluate as the callback and also disconnect the observer
during cleanup.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8d099b60-c5db-4aa3-886b-39add09b8e7b

📥 Commits

Reviewing files that changed from the base of the PR and between d6b2cf5 and 5805bfc.

📒 Files selected for processing (1)

• js/app/packages/app/component/split-layout/utils/createHeaderCollapser.ts

[coderabbitai]

⚠️ Potential issue | 🟠 Major

ResizeObserver stays bound to the first container instance.

If getContainer() later points to a new element, that new node is never observed, so collapse logic can silently stop reacting to resizes.

Proposed fix

 let observer: ResizeObserver | null = null;
+let observedContainer: HTMLElement | null = null;

 const evaluate = () => {
   const headerLeft = getContainer();
   if (!headerLeft) return;

-  if (!observer) {
-    observer = new ResizeObserver(() => scheduleEvaluate());
-    observer.observe(headerLeft);
-  }
+  if (!observer) {
+    observer = new ResizeObserver(() => scheduleEvaluate());
+  }
+  if (observedContainer !== headerLeft) {
+    if (observedContainer) observer.unobserve(observedContainer);
+    observer.observe(headerLeft);
+    observedContainer = headerLeft;
+  }
   ...
 };

 onCleanup(() => {
   observer?.disconnect();
+  observedContainer = null;
   if (rafId !== null) cancelAnimationFrame(rafId);
 });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@js/app/packages/app/component/split-layout/utils/createHeaderCollapser.ts`
around lines 65 - 68, In createHeaderCollapser the ResizeObserver instance
(observer) is only attached once to the initial header element and never
re-attached if getContainer() later returns a new node; update the logic so
whenever headerLeft is resolved you call observer.observe(headerLeft) (and if
the resolved element differs from the previously-observed node, call
observer.unobserve(prev) or observer.disconnect() first) and ensure you store
the currentlyObserved element so future container swaps rebind the observer;
keep using scheduleEvaluate as the callback and also disconnect the observer
during cleanup.