# Frontend Mentor — QR Code Component: Feedback

**Challenge:** QR Code Component (Newbie)
**Marker:** Antigravity AI
**Date:** 2026-04-04

---

## Overall Assessment: ✅ Solid Pass — 8/10

This is a well-executed first challenge. The component looks visually accurate, the right tools were reached for, and the code is clean and readable. A few refinements (mostly around box-shadow syntax, CSS best practices, and responsiveness) would push this to a polished solution.

---

## ✅ What Was Done Well

### 1. Accurate Visual Result
The rendered component closely matches both the desktop and mobile design references. The card is correctly centred, the QR image has matching border-radius, typography is right, and colours are clearly derived from the style guide.

### 2. Correct Font
`Outfit` was imported from Google Fonts with the correct weight range (`400..700`). Font-family is set at the `:root` level — good habit.

### 3. CSS Custom Properties
Using `:root` variables for colours and spacing is a great practice, especially on a component like this that could be reused or themed. Well done for reaching for this even at the newbie level.

```css
:root {
    --slate-bg: #d5e1ef;
    --slate-500: #68778d;
    --slate-900: #1f314f;
    --white: #ffffff;
}
```

### 4. Semantic HTML Structure
Using `<main>` as the layout wrapper and `<h2>` + `<p>` inside the card is reasonable and accessible. The `alt` attribute on the image (`"QR code image"`) is present — good.

### 5. `100dvh` / `100dvw`
Using `dvh`/`dvw` units for the full-page layout is a modern touch — it handles mobile browser chrome correctly (avoids the classic iOS Safari address bar issue with `100vh`).

---

## ❌ Issues & Areas to Improve

### 1. 🐛 Broken `box-shadow` Value (Bug)
**File:** `index.html`, line 59

```css
/* Current — BROKEN */
box-shadow: 0px 4px 4 0 rgba(0, 0, 0, 0.25);

/* Correct */
box-shadow: 0px 4px 4px 0px rgba(0, 0, 0, 0.25);
```

The third value `4` is missing its unit (`px`) and the spread value (`0`) is missing its unit too. Browsers silently discard an invalid `box-shadow` declaration, so the card renders **without a shadow at all**. Comparing against the design, the card should have a visible drop shadow beneath it.

> **Fix:** Add `px` to both unit-less values.

---

### 2. 🎨 Colour Values Don't Match the Style Guide
The style guide specifies colours in `hsl()` format. The implementation converts them to hex, which is fine functionally, but the hex values used are slightly off from the HSL spec:

| Token      | Style Guide (HSL)          | Used (hex)  | Match? |
|------------|----------------------------|-------------|--------|
| Slate 300  | `hsl(212, 45%, 89%)`       | `#d5e1ef`   | ~Close |
| Slate 500  | `hsl(216, 15%, 48%)`       | `#68778d`   | ❌ Off  |
| Slate 900  | `hsl(218, 44%, 22%)`       | `#1f314f`   | ~Close |

`hsl(216, 15%, 48%)` computes to approximately `#6b7a90`, not `#68778d`. This is a small difference, but a good habit is to either use the HSL values directly (which browsers fully support) or use a reliable converter.

> **Fix:** Use the HSL values from the style guide directly:
> ```css
> --slate-bg:  hsl(212, 45%, 89%);
> --slate-500: hsl(216, 15%, 48%);
> --slate-900: hsl(218, 44%, 22%);
> --white:     hsl(0, 0%, 100%);
> ```

---

### 3. 📐 Fixed `height` on the Card Is Fragile
**File:** `index.html`, line 57

```css
height: 499px; /* ❌ avoid this */
```

Hard-coding a pixel height is fragile. If a user zooms in, changes their browser font size, or content ever changes, the card will overflow or have incorrect spacing. Instead, let the content determine the card's height with padding.

> **Fix:** Remove `height: 499px` and rely on the grid's natural sizing + padding:
> ```css
> .card {
>     padding: 16px 16px 40px;
>     /* remove height: 499px */
> }
> ```

---

### 4. 📱 Responsiveness: Card Width on Small Screens
The card uses `width: 320px` (fixed). On a 320px viewport — the smallest commonly tested width — the card will touch the edges with zero horizontal margin, causing it to feel cramped or clip on some devices.

> **Fix:** Use `max-width` with a fluid fallback:
> ```css
> .card {
>     width: min(320px, calc(100% - 48px));
>     /* or */
>     max-width: 320px;
>     width: calc(100% - 48px);
> }
> ```

---

### 5. 📐 `grid-template-columns: 1` — Invalid Value
**File:** `index.html`, line 51

```css
grid-template-columns: 1; /* ❌ invalid */
```

`1` is not a valid CSS grid column value. The browser ignores this declaration. While it doesn't visually break anything here (the card only has one column anyway), it's incorrect syntax and would cause issues in more complex grids.

> **Fix:** Remove the property entirely (single-column is the default), or use a valid value:
> ```css
> grid-template-columns: 1fr;
> ```

---

### 6. 💡 Minor: `margin: 0 auto` on Inline Elements Isn't Needed
**File:** `index.html`, lines 73 & 82

```css
.card h2, .card p {
    margin: 0 auto; /* redundant for block elements with text-align: center */
}
```

`h2` and `p` are block-level elements that naturally take full width. Since the card uses `text-align: center`, centering the text works without `margin: 0 auto`. Harmless, but unnecessary.

---

## 📊 Scoring Breakdown

| Category               | Score | Notes |
|------------------------|-------|-------|
| Visual accuracy        | 9/10  | Very close — only the missing shadow hurts |
| Code quality / CSS     | 6/10  | Fixed height, invalid grid value, broken shadow, off-spec hex colours |
| Responsiveness         | 7/10  | Works on most sizes, but could clip at 320px |
| Semantic HTML          | 9/10  | Good use of `<main>`, proper `alt` text |
| Best practices         | 8/10  | CSS vars and `dvh` units are great touches |
| **Overall**            | **8/10** | |

---

## 🔧 Suggested Revised CSS (Key Fixes)

```css
:root {
    --slate-bg:  hsl(212, 45%, 89%);
    --slate-500: hsl(216, 15%, 48%);
    --slate-900: hsl(218, 44%, 22%);
    --white:     hsl(0, 0%, 100%);
    font-family: "Outfit", sans-serif;
}

.card {
    background-color: var(--white);
    border-radius: 20px;
    display: grid;
    grid-template-rows: auto auto auto;
    gap: 24px;
    padding: 16px 16px 40px;
    max-width: 320px;
    width: calc(100% - 48px); /* fluid on small screens */
    text-align: center;
    box-shadow: 0px 25px 25px 0px rgba(0, 0, 0, 0.05); /* visible shadow */
}
```

---

## 💬 Final Thoughts

This is a strong start for a newbie challenge. The visual output is close to pixel-perfect and you clearly understood the brief. The main takeaways are:

1. **Always validate your CSS values** — missing units (`px`) in `box-shadow` silently break entire declarations.
2. **Prefer `max-width` over `width` for components** — it's immediately more responsive.
3. **Use the style guide's units directly** — HSL is CSS-native; no need to convert to hex.
4. **Avoid hard-coding heights** — let padding and content breathe.

Keep building! 🚀

---

*This feedback was generated by **Antigravity AI**. The challenge solution was written entirely by the developer — AI was only used for evaluation after the fact.*