diff --git a/.claude/PRPs/issues/completed/issue-397.md b/.claude/PRPs/issues/completed/issue-397.md new file mode 100644 index 0000000..fda453f --- /dev/null +++ b/.claude/PRPs/issues/completed/issue-397.md @@ -0,0 +1,250 @@ +Hello tbrandenburg! + +Thanks for your feedback! + +Here are the tools I am offering: + +**Commands:** +/ghar-commit-push +/ghar-issue-fix +/ghar-issue-investigate +/ghar-maintainability-review +/ghar-resolve-ci-errors +/ghar-review +/ghar-security-check +/ghar-stale-check + +**Agents:** +*review + +**Usage:** +- Use commands directly: `/command-name` +- Use agents with commands: `*agent-name /command-name PROMPT` + +Example: `*review /security-review Please check this code for vulnerabilities` +# Investigation: sidebar-version margin-top uses magic value that may break with header style changes + +**Issue**: #397 (https://github.com/tbrandenburg/made/issues/397) +**Type**: BUG +**Investigated**: 2026-04-28T12:00:00Z + +### Assessment + +| Metric | Value | Reasoning | +| ---------- | ------ | ----------------------------------------------------------------------------------------------------------------------------------- | +| Severity | LOW | Cosmetic CSS issue that only manifests if header styles change; no current user impact since .sidebar-version doesn't exist yet. | +| Complexity | LOW | Single CSS file change (sidebar.css); no integration points or architectural changes needed. | +| Confidence | HIGH | The problematic CSS is explicitly quoted in the issue (#300's proposal); the fix is clearly specified and the codebase patterns are clear. | + +--- + +## Problem Statement + +The `.sidebar-version` CSS rule proposed in issue #300 uses `margin-top: -1rem` to tighten spacing with the `.sidebar-header` above it. This negative margin is a magic value that depends implicitly on the header's bottom padding/margin. If `.sidebar-header` styles change (font-size, padding, spacing), the version label could overlap the header or leave an unexpected gap. + +--- + +## Analysis + +### Root Cause / Change Rationale + +The root cause is a CSS design choice that creates an implicit coupling between `.sidebar-version` and `.sidebar-header` via a negative margin. The fix is to make spacing explicit by controlling it on `.sidebar-header` with `padding-bottom` and removing the negative margin from `.sidebar-version`. + +### Evidence Chain + +WHY: `.sidebar-version` may overlap `.sidebar-header` or leave unexpected gap if header styles change +↓ BECAUSE: `.sidebar-version` uses `margin-top: -1rem` which is a magic value tied to current header spacing +Evidence: Issue #397 body - `.sidebar-version { margin-top: -1rem; }` + +↓ BECAUSE: The negative margin compensates for unspecified bottom spacing on `.sidebar-header` +Evidence: `packages/frontend/src/styles/sidebar.css:15-18` - `.sidebar-header` has no explicit bottom padding/margin + +↓ ROOT CAUSE: Spacing between header and version is controlled by a negative margin on the child rather than explicit padding on the parent +Evidence: Issue #397 proposed solution - move spacing control to `.sidebar-header` with `padding-bottom: 0.25rem` + +### Affected Files + +| File | Lines | Action | Description | +| ----------------------------------------- | ----- | ------ | -------------------------------------------------------------------------------- | +| `packages/frontend/src/styles/sidebar.css` | 15-18 | UPDATE | Add `padding-bottom: 0.25rem` to `.sidebar-header` | +| `packages/frontend/src/styles/sidebar.css` | NEW | UPDATE | Add `.sidebar-version` class without negative margin (when version feature added) | + +### Integration Points + +- `.sidebar-header` is used in `packages/frontend/src/components/Sidebar.tsx:38` +- The version display will be added to the Sidebar component (per issue #300) +- No other files depend on these CSS classes currently + +### Git History + +- **Introduced**: Not yet introduced (issue #300 proposed the CSS, not yet merged) +- **Last modified**: `21512a2` - Merge pull request #394 (sidebar.css not changed) +- **Implication**: This is a preemptive fix for code that doesn't exist yet; apply when implementing issue #300 + +--- + +## Implementation Plan + +### Step 1: Add padding-bottom to .sidebar-header + +**File**: `packages/frontend/src/styles/sidebar.css` +**Lines**: 15-18 +**Action**: UPDATE + +**Current code:** + +```css +.sidebar-header { + font-weight: 700; + font-size: 1.5rem; +} +``` + +**Required change:** + +```css +.sidebar-header { + font-weight: 700; + font-size: 1.5rem; + padding-bottom: 0.25rem; +} +``` + +**Why**: Explicitly controls bottom spacing of header, making it resilient to style changes. + +--- + +### Step 2: Add .sidebar-version class without negative margin + +**File**: `packages/frontend/src/styles/sidebar.css` +**Lines**: After `.sidebar-header` block +**Action**: UPDATE (add new rule) + +**Current code:** + +```css +.sidebar-header { + font-weight: 700; + font-size: 1.5rem; + padding-bottom: 0.25rem; +} +``` + +**Required change:** + +```css +.sidebar-header { + font-weight: 700; + font-size: 1.5rem; + padding-bottom: 0.25rem; +} + +.sidebar-version { + font-size: 0.7rem; + text-align: center; + color: var(--muted); + margin-top: 0; + margin-bottom: 0.5rem; +} +``` + +**Why**: Removes the magic `margin-top: -1rem` value and makes spacing explicit. The `margin-bottom: 0.5rem` provides consistent spacing below the version label. + +--- + +### Step 3: Add version display to Sidebar component (when implementing #300) + +**File**: `packages/frontend/src/components/Sidebar.tsx` +**Lines**: After line 38 +**Action**: UPDATE + +**Current code:** + +```tsx +
MADE
+