fix(frontend): keep KnowledgeBase modal centered when side panel opens

[AntonioABLima]

Summary

• When a file is added in the Knowledge Base Upload Modal, the side panel (300 px) was sliding in to the right, shifting the combined modal 150 px off-center visually
• Fixed by translating the DialogContent left by half the side panel width (-150px) using the CSS translate property — separate from transform so it doesn't conflict with the existing zoom-in/out keyframe animation
• Unified SidePanel animation timing to duration-300 ease-in-out so both the dialog shift and panel slide-in are fully in sync

Refactors included (no behavior change):

• isPanelOpen replaces the duplicated sidePanel && sidePanelOpen checks in StepperModal
• modalHeight / modalBase / errorCount replace the IIFE inside the JSX height prop in KnowledgeBaseUploadModal
• showHelpButton replaces the repeated !hideAdvanced && currentStep === 1 condition

Scope: StepperModal has a single consumer (KnowledgeBaseUploadModal), so no other flows are affected.

Before

Modal.Errado.Cut.mp4

After

Modal.CorretoCut.mp4

Test plan

• Open the Knowledge Base Upload Modal
• Add one or more files — side panel slides in and the combined modal stays visually centered
• Remove all files — side panel closes and modal re-centers with a smooth animation
• Open/close the modal normally — zoom-in/out animation is unaffected
• Jest: StepperModal.test.tsx, KnowledgeBaseUploadModal.test.tsx — 86 tests passing ✅

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b52c8a65-acfe-4d8c-91e6-01dba9f50904

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/knowledge-base-modal-center-on-side-panel

---

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.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 52.79%. Comparing base (3ea28ed) to head (eb1a25a).
⚠️ Report is 1 commits behind head on release-1.10.0.

❌ Your project status has failed because the head coverage (49.82%) is below the target coverage (60.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

@@                Coverage Diff                @@
##           release-1.10.0   #12764     +/-   ##
=================================================
  Coverage           52.79%   52.79%             
=================================================
  Files                2025     2025             
  Lines              184228   184241     +13     
  Branches            27403    28910   +1507     
=================================================
+ Hits                97264    97273      +9     
- Misses              85869    85875      +6     
+ Partials             1095     1093      -2     

Flag	Coverage Δ
backend	55.93% <ø> (+0.02%)	⬆️
frontend	52.78% <100.00%> (-0.01%)	⬇️
lfx	49.82% <ø> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...wledgeBaseUploadModal/KnowledgeBaseUploadModal.tsx	100.00% <100.00%> (+1.92%)	⬆️
...d/src/modals/knowledgeBaseUploadModal/constants.ts	100.00% <100.00%> (ø)
.../frontend/src/modals/stepperModal/StepperModal.tsx	96.59% <100.00%> (+0.11%)	⬆️
...d/src/modals/stepperModal/components/SidePanel.tsx	90.74% <100.00%> (ø)
src/frontend/src/modals/stepperModal/constants.ts	100.00% <100.00%> (ø)

... and 17 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

Frontend Unit Test Coverage Report

Coverage Summary

Lines	Statements	Branches	Functions
	34.75% (39869/114724)	67.39% (5443/8076)	35.73% (932/2608)

Unit Test Results

Tests	Skipped	Failures	Errors	Time
3824	0 💤	0 ❌	0 🔥	7m 29s ⏱️

[Cristhianzl]

1. Executive Summary

Verdict	Comment
🟠 Request changes	Non-blocking UX fix with a genuine DRY/magic-number issue and missing regression coverage. Security & structure checks pass. Minor polish needed before merge.

The PR fixes a real UX bug (combined dialog visually off-center after the 300 px side panel slides in) via a CSS-only translate, plus three small local-scope refactors. Implementation is small and surgical. However:

1. The -150px hardcoded shift is decoupled from SIDE_PANEL_WIDTH (w-[300px]) — if the panel width ever changes, the modal will stop being centered. DRY / magic-number violation.
2. No test was added for the new translate / isPanelOpen behavior. The PR description claims "86 tests passing" but those are all pre-existing — none asserts the fix itself.
3. Tailwind animation syntax was normalized (transition-opacity → transition-[opacity], transition-transform → transition-[transform]) — not wrong, but noise in the diff.

---

2. Security Review Mindset

Question	Finding
What is this code trusting without verifying?	Nothing — purely presentational CSS translate + derived booleans.
What is the authoritative source?	Tailwind CSS translate utility + standard React props. No third-party integration.
What happens in the failure path?	If sidePanel prop is falsy, isPanelOpen is false, translate: "0 0" — modal renders centered as before. Safe default.
Who controls each value, could they lie?	sidePanelOpen / sidePanel are internal props set by the single consumer (KnowledgeBaseUploadModal). No user-controlled input.
Blast radius if wrong?	Cosmetic only — modal could be mis-centered. No data, authz, or secret exposure.

🔴 CRITICAL security checklist: ✅ pass (no PII, no secrets, no LLM/AI, no third-party integration, no DB access, no logging).

---

3. File-by-File Review

3.1 src/frontend/src/modals/stepperModal/StepperModal.tsx

+  const isPanelOpen = Boolean(sidePanel) && sidePanelOpen;
 ...
-          style={heightStyle}
+          style={{
+            ...heightStyle,
+            translate: isPanelOpen ? "-150px 0" : "0 0",
+          }}
           className={cn(
-            "... transition-[height,width,border-radius,opacity] duration-300 ease-in-out",
+            "... transition-[height,width,border-radius,opacity,translate] duration-300 ease-in-out",
 ...
-            sidePanel && sidePanelOpen
+            isPanelOpen

Findings:

#	Severity	Finding
F1	🟠 IMPORTANT — Magic number / DRY	-150px is half of the side panel's w-[300px]. The constant exists (SIDE_PANEL_WIDTH in stepperModal/constants.ts:14) but is hardcoded as the Tailwind class "w-[300px]", so no numeric source of truth is reused. If the panel width ever changes, this translate will silently desync. Suggested fix: export a numeric SIDE_PANEL_WIDTH_PX = 300 alongside the Tailwind class and compute translate: \${-SIDE_PANEL_WIDTH_PX / 2}px 0``.
F2	🟡 RECOMMENDED — KISS / style merge	style={{ ...heightStyle, translate: ... }} — when heightStyle is undefined, spread still works but returns a translate-only object. Fine, just worth a comment that heightStyle may be undefined; current code handles it correctly via the spread.
F3	🟢 NICE TO HAVE — Naming	isPanelOpen is clear, but hasSidePanelOpen or isSidePanelVisible would be slightly less ambiguous (there's also a sidePanelOpen prop right above it, which reads as "is the prop truthy" vs "is there a panel AND is it open"). Minor.
F4	🟡 RECOMMENDED — Animation coupling	translate is listed in transition-[height,width,border-radius,opacity,translate] with duration-300. Good — this is the reason the panel and the shift are in sync. Verify the transition-[translate] utility is supported on the project's Tailwind version (Tailwind ≥ 3.3).
F5	🟢 OK	SRP preserved — file still has a single responsibility (render stepper dialog).
F6	🟢 OK	LOC: 128 lines — well under 500-line limit.

3.2 src/frontend/src/modals/stepperModal/components/SidePanel.tsx

-        "... transition-opacity duration-150",
+        "... transition-[opacity] duration-300 ease-in-out",
...
-            "h-full transition-transform duration-300 ease-out",
+            "h-full transition-[transform] duration-300 ease-in-out",

Findings:

#	Severity	Finding
F7	🟡 RECOMMENDED — Behavior change beyond the stated fix	The opacity transition duration doubled (150 ms → 300 ms) and its easing was changed. This is justified by the PR description ("Unified SidePanel animation timing ... in sync"), but worth flagging: the side panel's fade-in is now perceptibly slower. Please verify visually and with accessibility (prefers-reduced-motion) that the new timing still feels snappy.
F8	🟢 OK — Tailwind syntax normalization	transition-opacity → transition-[opacity] and transition-transform → transition-[transform] are semantically equivalent for Tailwind. Switching to the arbitrary-value form is consistent with the expanded list in StepperModal.tsx, but it is a style change that increases diff churn. Either form is fine — recommend sticking to the shorthand utilities (transition-opacity) when listing a single property.

3.3 src/frontend/src/modals/knowledgeBaseUploadModal/KnowledgeBaseUploadModal.tsx

+  const errorCount = Object.keys(form.validationErrors).length;
+  const modalBase = !hideAdvanced && form.showAdvanced ? 690 : 347;
+  const modalHeight = `${modalBase + errorCount * 16}`;
+
+  const showHelpButton = !hideAdvanced && form.currentStep === 1;
 ...
-      height={(() => {
-        const errorCount = ...
-        return `${base + errorCount * 16}`;
-      })()}
+      height={modalHeight}
 ...
-          helpLabel={ !hideAdvanced && form.currentStep === 1 ? ... : undefined }
-          onHelp={ !hideAdvanced && form.currentStep === 1 ? ... : undefined }
+          helpLabel={ showHelpButton ? ... : undefined }
+          onHelp={ showHelpButton ? ... : undefined }

Findings:

#	Severity	Finding
F9	🟢 OK — DRY	Good refactor. The duplicated !hideAdvanced && form.currentStep === 1 expression becomes showHelpButton at a single source.
F10	🟠 IMPORTANT — Magic numbers	690, 347, 16 still appear in the extracted modalBase / modalHeight expressions. These existed before this PR (pre-existing), but since the author is already touching these lines to extract the variables, it's a cheap follow-up to extract: MODAL_HEIGHT_WITH_ADVANCED = 690, MODAL_HEIGHT_DEFAULT = 347, VALIDATION_ERROR_LINE_PX = 16. Per BUG_FIXES_RULE, "pre-existing issue in the same lines you're touching and trivial to fix → fix it in the same PR".
F11	🟢 OK	LOC: 151 lines — well under 500.
F12	🟢 OK	SRP preserved — single modal orchestration component.

---

4. REVIEWER_RULE Checklist

🔴 CRITICAL (Blockers)

Item	Status
No PII in any logs / prints / webhook messages	✅ N/A — no logging
No secrets/credentials in code	✅
No duplicate types, classes, or logic (DRY)	🟠 See F1 & F10 (magic numbers shared across files)
No file exceeds 500 lines	✅ — all modified files ≤ 151 lines
No file has > 5 functions with DIFFERENT responsibilities	✅
No file has > 10 functions even with same responsibility	✅
No file has > 1 main class	✅
No mixed responsibility prefixes in same file	✅

🔴 CRITICAL — AI/Chatbot Features

✅ N/A — no AI / LLM / agent code.

🔴 CRITICAL — Third-Party Integration Security

✅ N/A — no external integration.

🟠 IMPORTANT

Item	Status
SRP respected	✅
No growing if/elif chains (OCP)	✅
LSP / ISP / DIP	✅ N/A for this presentational layer
YAGNI	✅
Law of Demeter	✅ (form.validationErrors, form.showAdvanced are 1-level accesses through a hook result — acceptable)
Strong typing (no any / object)	✅
Inputs validated at boundaries	✅ N/A — internal props
Error handling	✅ N/A
Types/constants in dedicated files	⚠️ Partial — SIDE_PANEL_WIDTH is in constants.ts but only as a Tailwind class, not as a reusable numeric constant (see F1). Modal height magic numbers (690, 347, 16) not extracted (see F10).

🟡 RECOMMENDED

Item	Status
Appropriate logging at key points	✅ N/A
No unnecessary comments	✅
No over-engineering	✅

🟢 TESTING

Item	Status
Unit tests for core logic	⚠️ Partial — existing tests cover rendering, but no test asserts the new translate / isPanelOpen behavior introduced by this PR
Tests cover success, error, and edge cases	⚠️ Happy path only for the fix
BOTH happy path AND adversarial tests	❌ No test for: sidePanel truthy + sidePanelOpen=false (no translate), sidePanel=undefined + sidePanelOpen=true (no translate — Boolean(sidePanel) short-circuit), or the class-name transition rounded-l-xl rounded-r-none
Coverage ran and shown ≥ 75%	❌ No coverage output posted on this PR
All created tests pass — zero failures	✅ 86 pre-existing tests reported passing
Not prolonging legacy bad patterns	⚠️ Leaves KnowledgeBaseUploadModal.test.tsx at 842 lines (pre-existing violation of 500-line limit — out of scope for this PR but worth a follow-up ticket)

---

5. Required Changes (🟠 Before Merge)

1. Extract the numeric side-panel width to eliminate the -150px magic number (F1).
   • Add to stepperModal/constants.ts:

     export const SIDE_PANEL_WIDTH_PX = 300;
     export const SIDE_PANEL_WIDTH = `w-[${SIDE_PANEL_WIDTH_PX}px]`;

     (or, pragmatically, export a separate SIDE_PANEL_HALF_OFFSET_PX = 150)
   • Replace in StepperModal.tsx:

     translate: isPanelOpen ? `${-SIDE_PANEL_WIDTH_PX / 2}px 0` : "0 0",

2. Add a regression test in stepperModal/__tests__/StepperModal.test.tsx asserting:
   • translate: "-150px 0" (or the derived value) when sidePanel is provided and sidePanelOpen is true.
   • translate: "0 0" when sidePanel is omitted OR sidePanelOpen is false.
   • The rounded-l-xl rounded-r-none border-r-0 classes appear iff isPanelOpen.

6. Suggested Follow-ups (🟡 Optional, same PR or a separate ticket)

3. Extract modal height magic numbers (690, 347, 16) in KnowledgeBaseUploadModal.tsx to named constants in knowledgeBaseUploadModal/constants.ts (F10).
4. Revert Tailwind arbitrary-value transitions in SidePanel.tsx back to shorthand (transition-opacity, transition-transform) to reduce diff churn — the behavior change that matters is the duration-300 unification, not the syntax.
5. Verify visually the slower side-panel fade (150 ms → 300 ms) still feels responsive, and that the translate animation respects prefers-reduced-motion (F7).

Dismissing this pending review since the issue has already been resolved. Thanks!

wrong account

[Cristhianzl]

lgtm