Skip to content

Fix: sidebar-version margin-top uses magic value (#397)#398

Merged
tbrandenburg merged 2 commits intomainfrom
fix/issue-397-sidebar-version-margin
Apr 28, 2026
Merged

Fix: sidebar-version margin-top uses magic value (#397)#398
tbrandenburg merged 2 commits intomainfrom
fix/issue-397-sidebar-version-margin

Conversation

@github-actions
Copy link
Copy Markdown

Summary

The .sidebar-version CSS rule uses margin-top: -1rem which is a magic value that creates an implicit coupling with .sidebar-header spacing. If header styles change, the version label could overlap or leave unexpected gaps.

Root Cause

Spacing between header and version was controlled by a negative margin on the child (.sidebar-version) rather than explicit padding on the parent (.sidebar-header).

Changes

File Change
packages/frontend/src/styles/sidebar.css Added padding-bottom: 0.25rem to .sidebar-header for explicit spacing control
packages/frontend/src/styles/sidebar.css Removed negative margin from .sidebar-version, added margin-bottom: 0.5rem

Testing

  • Lint passes (npm run lint)
  • CSS changes follow codebase patterns (using var(--muted) and consistent spacing values)

Validation

cd packages/frontend && npm run lint

Issue

Fixes #397


📋 Implementation Details

Implementation followed artifact:

Investigation artifact from issue #397

Deviations from plan:

None - implemented exactly as specified in the artifact.


Automated implementation from investigation artifact

…header style changes (#397)

The .sidebar-version CSS rule uses margin-top: -1rem which is a magic value
that creates an implicit coupling with .sidebar-header spacing.

Changes:
- Added padding-bottom: 0.25rem to .sidebar-header for explicit spacing control
- Removed negative margin from .sidebar-version and added margin-bottom: 0.5rem

Fixes #397
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 28, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
made Ready Ready Preview, Comment Apr 28, 2026 0:16am

@github-actions
Copy link
Copy Markdown
Author

🔍 Automated Code Review

Summary

The fix removes the magic margin-top: -1rem value from .sidebar-version and adds explicit spacing control. However, there's a concern about interaction with the flex gap: 1.5rem on .sidebar.

Findings

✅ Strengths

  • Successfully removes the problematic magic value margin-top: -1rem
  • Uses codebase patterns: var(--muted) for secondary text, consistent spacing values
  • Lint passes

⚠️ Suggestions (non-blocking)

  • packages/frontend/src/styles/sidebar.css:65-70 - The .sidebar container uses flexbox with gap: 1.5rem. Adding padding-bottom: 0.25rem to .sidebar-header and margin-bottom: 0.5rem to .sidebar-version may create double spacing with the flex gap. Consider whether these are needed given the flex gap handles spacing between children.

  • packages/frontend/src/styles/sidebar.css:65-70 - If the goal is simply to remove the negative margin, the simpler fix could be:

    .sidebar-version {
      font-size: 0.7rem;
      text-align: center;
      color: var(--muted);
      /* Let flex gap handle spacing */
    }

    And revert the padding-bottom addition to .sidebar-header.

🔒 Security

  • No security concerns identified (CSS-only change with no dynamic content)

Checklist

  • Fix addresses root cause (removes magic negative margin)
  • Code follows codebase patterns
  • Lint passes
  • Consider flex gap interaction for optimal spacing

Self-reviewed by opencode • Ready for human review

@tbrandenburg tbrandenburg merged commit e0de34a into main Apr 28, 2026
2 checks passed
@tbrandenburg tbrandenburg deleted the fix/issue-397-sidebar-version-margin branch April 28, 2026 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sidebar-version margin-top uses magic value that may break with header style changes

1 participant