Extract magic number to named constant in keyboard navigation #17

2025-11-17T09:01:00-06:00

Copilot commented

2025-11-17 09:01:00 -06:00

(Migrated from github.com)

The keyboard navigation column calculation used a magic number 400 without documenting its relationship to the CSS grid layout (minmax(300px, 1fr)).

Changes

Extracted 400 to CARD_WIDTH_WITH_GAP constant with documentation linking it to CSS grid settings
Replaced inline calculation with named constant

// Before
const cols = Math.floor(window.innerWidth / 400); // Approximate columns

// After
// Grid layout constants
// This should match the CSS grid-template-columns: repeat(auto-fit, minmax(300px, 1fr))
// The value accounts for minimum card width (300px) + gap (40px) + padding considerations
const CARD_WIDTH_WITH_GAP = 400;

const cols = Math.floor(window.innerWidth / CARD_WIDTH_WITH_GAP);

This makes the relationship between JavaScript navigation logic and CSS grid layout explicit, improving maintainability if grid settings change.

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

The keyboard navigation column calculation used a magic number `400` without documenting its relationship to the CSS grid layout (`minmax(300px, 1fr)`). ## Changes - Extracted `400` to `CARD_WIDTH_WITH_GAP` constant with documentation linking it to CSS grid settings - Replaced inline calculation with named constant ```javascript // Before const cols = Math.floor(window.innerWidth / 400); // Approximate columns // After // Grid layout constants // This should match the CSS grid-template-columns: repeat(auto-fit, minmax(300px, 1fr)) // The value accounts for minimum card width (300px) + gap (40px) + padding considerations const CARD_WIDTH_WITH_GAP = 400; const cols = Math.floor(window.innerWidth / CARD_WIDTH_WITH_GAP); ``` This makes the relationship between JavaScript navigation logic and CSS grid layout explicit, improving maintainability if grid settings change.  --- ✨ Let Copilot coding agent [set things up for you](https://github.com/SluberskiHomeLab/castroix/issues/new?title=✨+Set+up+Copilot+instructions&body=Configure%20instructions%20for%20this%20repository%20as%20documented%20in%20%5BBest%20practices%20for%20Copilot%20coding%20agent%20in%20your%20repository%5D%28https://gh.io/copilot-coding-agent-tips%29%2E%0A%0A%3COnboard%20this%20repo%3E&assignees=copilot) — coding agent works faster and does higher quality work when set up for your repo.

SluberskiHomeLab (Migrated from github.com) reviewed

2025-11-17 09:01:00 -06:00

copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed

2025-11-17 09:31:08 -06:00

copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment

Pull Request Overview

This PR extracts a magic number (400) used in keyboard navigation column calculations into a well-named constant (CARD_WIDTH_WITH_GAP) with documentation explaining its relationship to the CSS grid layout. This improves code maintainability by making the connection between JavaScript navigation logic and CSS grid configuration explicit.

Key changes:

Added CARD_WIDTH_WITH_GAP constant with documentation linking it to CSS grid settings
Replaced inline magic number with the named constant in keyboard navigation logic

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

## Pull Request Overview This PR extracts a magic number (`400`) used in keyboard navigation column calculations into a well-named constant (`CARD_WIDTH_WITH_GAP`) with documentation explaining its relationship to the CSS grid layout. This improves code maintainability by making the connection between JavaScript navigation logic and CSS grid configuration explicit. **Key changes:** - Added `CARD_WIDTH_WITH_GAP` constant with documentation linking it to CSS grid settings - Replaced inline magic number with the named constant in keyboard navigation logic --- 💡 <a href="/SluberskiHomeLab/castroix/new/main/.github/instructions?filename=*.instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add Copilot custom instructions</a> for smarter, more guided reviews. <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn how to get started</a>.

renderer.js

					
				@ -5,1 +5,4 @@

				// Grid layout constants

				// This should match the CSS grid-template-columns: repeat(auto-fit, minmax(300px, 1fr))

				// The value accounts for minimum card width (300px) + gap (40px) + padding considerations

The comment mentions "padding considerations" but it's unclear how this contributes to the 400px value. The CSS shows: minimum card width (300px) + gap (40px) = 340px. The grid container has padding: 40px 80px which applies to the container edges, not per-column spacing. Consider clarifying what specifically accounts for the remaining 60px (400 - 340) in this calculation, or if 400px is an intentional approximation/buffer to prevent edge cases where cards might wrap unexpectedly.

// The value is minimum card width (300px) + gap (40px) = 340px, plus a buffer (60px) to prevent edge cases where cards might wrap unexpectedly. Container padding does not affect per-column spacing.

The comment mentions "padding considerations" but it's unclear how this contributes to the 400px value. The CSS shows: minimum card width (300px) + gap (40px) = 340px. The grid container has `padding: 40px 80px` which applies to the container edges, not per-column spacing. Consider clarifying what specifically accounts for the remaining 60px (400 - 340) in this calculation, or if 400px is an intentional approximation/buffer to prevent edge cases where cards might wrap unexpectedly. ```suggestion // The value is minimum card width (300px) + gap (40px) = 340px, plus a buffer (60px) to prevent edge cases where cards might wrap unexpectedly. Container padding does not affect per-column spacing. ```

Sign in to join this conversation.

No reviewers

SluberskiHomeLab