fix: use 100dvh for mobile sign-in page centering

forgejo_admin commented

2026-03-28 22:47:21 +00:00

Owner

Summary

On 390px mobile viewports, /signin and /forgot-password pages render with content pushed to the bottom half. 100vh includes mobile browser chrome (URL bar), making the container taller than the visible area and breaking vertical centering. Switching to 100dvh (dynamic viewport height) accounts for browser chrome.

Changes

src/app.css — .signin-page: changed min-height: 100vh to min-height: 100dvh
src/routes/(app)/forgot-password/+page.svelte — .forgot-page: changed min-height: 100vh to min-height: 100dvh

Review Checklist

Two files changed, single property value each
No logic changes, CSS-only fix
dvh has broad browser support (Chrome 108+, Safari 15.4+, Firefox 94+)
Desktop unaffected — dvh equals vh when no browser chrome is present

Test Plan

Open /signin on a 390px mobile viewport (or real phone) — content should be vertically centered
Open /forgot-password on a 390px mobile viewport — content should be vertically centered
Verify desktop layout is unaffected

None

Forgejo issue: #154
Closes #154

## Summary On 390px mobile viewports, `/signin` and `/forgot-password` pages render with content pushed to the bottom half. `100vh` includes mobile browser chrome (URL bar), making the container taller than the visible area and breaking vertical centering. Switching to `100dvh` (dynamic viewport height) accounts for browser chrome. ## Changes - `src/app.css` — `.signin-page`: changed `min-height: 100vh` to `min-height: 100dvh` - `src/routes/(app)/forgot-password/+page.svelte` — `.forgot-page`: changed `min-height: 100vh` to `min-height: 100dvh` ## Review Checklist - [x] Two files changed, single property value each - [x] No logic changes, CSS-only fix - [x] `dvh` has broad browser support (Chrome 108+, Safari 15.4+, Firefox 94+) - [x] Desktop unaffected — dvh equals vh when no browser chrome is present ## Test Plan - Open `/signin` on a 390px mobile viewport (or real phone) — content should be vertically centered - Open `/forgot-password` on a 390px mobile viewport — content should be vertically centered - Verify desktop layout is unaffected ## Related Notes - None ## Related - Forgejo issue: #154 - Closes #154

forgejo_admin added 1 commit

2026-03-28 22:47:21 +00:00

fix: use 100dvh for mobile sign-in page centering

ci/woodpecker/pr/woodpecker Pipeline was successful

Details

e849daf969

100vh includes mobile browser chrome (URL bar), pushing content to
the bottom half on 390px devices. 100dvh (dynamic viewport height)
accounts for this.

Closes #154

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

forgejo_admin commented

2026-03-28 22:49:46 +00:00

Author

Owner

PR #161 Review

DOMAIN REVIEW

Tech stack: SvelteKit / CSS (static landing site)

CSS change analysis:
The diff is minimal and precise. Two files, one property change each:

src/app.css line 2686: .signin-page min-height: 100vh -> min-height: 100dvh
src/routes/(app)/forgot-password/+page.svelte line 87: .forgot-page min-height: 100vh -> min-height: 100dvh

Both selectors use the same flex centering pattern (display: flex; align-items: center; justify-content: center; min-height: ...), which is exactly the pattern broken by mobile browser chrome. The fix is correct.

dvh browser support: Chrome 108+ (Dec 2022), Safari 15.4+ (Mar 2022), Firefox 94+ (Nov 2021). Effectively universal for the target audience (parents on modern phones). Desktop is unaffected -- dvh equals vh when no browser chrome is present.

No logic changes, no JS changes, no new dependencies. This is a pure CSS property value swap.

BLOCKERS

None.

This is a 2-line CSS fix with no logic, no user input, no auth paths, and no new functionality. None of the BLOCKER criteria apply:

No new functionality requiring test coverage (CSS property swap)
No user input handling
No secrets or credentials
No auth/security path changes

NITS

Fallback for older browsers (low risk): If a browser does not understand dvh, the entire min-height declaration is silently dropped. A defensive pattern would be:
```
min-height: 100vh;      /* fallback */
min-height: 100dvh;     /* modern override */
```
Given the target audience and support dates (3+ years old), this is cosmetic rather than practical. Non-blocking.
Discovered scope -- other pages with the same 100vh pattern: The repo has 6 additional 100vh centering containers that will exhibit the same mobile bug:
- src/app.css line 3372: .auth-page (flex centered, same pattern)
- src/routes/(app)/reset-password/+page.svelte line 155: .reset-page
- src/routes/(app)/checkout/cancel/+page.svelte line 32: .cancel-page
- src/routes/(app)/checkout/success/+page.svelte line 17: .success-page
- src/routes/(app)/checkout/+page.svelte line 258: .checkout-page (padding-only, less impacted)
- src/routes/(app)/+layout.svelte line 83: inline style on loading spinner div
This is not a blocker for this PR -- the PR correctly fixes the two pages specified in issue #154. But these other pages should be tracked as discovered scope in a new Forgejo issue.

SOP COMPLIANCE

Branch named after issue: 154-fix-mobile-signin-centering references issue #154
PR body follows template: Summary, Changes, Test Plan, Related all present
Related references issue: Closes #154
Related references plan slug: N/A (bug fix, no plan)
No secrets committed
No scope creep -- changes are tightly scoped to the two pages in the issue
Commit message is descriptive (fix: use 100dvh for mobile sign-in page centering)

PROCESS OBSERVATIONS

Change failure risk: Very low. CSS-only change, two properties, well-understood fix for a well-documented browser behavior difference.
Discovered scope: The 6 other 100vh flex-centered containers should get their own issue to avoid the same bug report recurring for different pages.
No test plan gap: CSS viewport unit changes are not testable via unit tests. The PR body correctly specifies manual verification steps (390px viewport, real phone).

VERDICT: APPROVED

## PR #161 Review ### DOMAIN REVIEW **Tech stack:** SvelteKit / CSS (static landing site) **CSS change analysis:** The diff is minimal and precise. Two files, one property change each: - `src/app.css` line 2686: `.signin-page` `min-height: 100vh` -> `min-height: 100dvh` - `src/routes/(app)/forgot-password/+page.svelte` line 87: `.forgot-page` `min-height: 100vh` -> `min-height: 100dvh` Both selectors use the same flex centering pattern (`display: flex; align-items: center; justify-content: center; min-height: ...`), which is exactly the pattern broken by mobile browser chrome. The fix is correct. **dvh browser support:** Chrome 108+ (Dec 2022), Safari 15.4+ (Mar 2022), Firefox 94+ (Nov 2021). Effectively universal for the target audience (parents on modern phones). Desktop is unaffected -- `dvh` equals `vh` when no browser chrome is present. **No logic changes, no JS changes, no new dependencies.** This is a pure CSS property value swap. ### BLOCKERS None. This is a 2-line CSS fix with no logic, no user input, no auth paths, and no new functionality. None of the BLOCKER criteria apply: - No new functionality requiring test coverage (CSS property swap) - No user input handling - No secrets or credentials - No auth/security path changes ### NITS 1. **Fallback for older browsers (low risk):** If a browser does not understand `dvh`, the entire `min-height` declaration is silently dropped. A defensive pattern would be: ```css min-height: 100vh; /* fallback */ min-height: 100dvh; /* modern override */ ``` Given the target audience and support dates (3+ years old), this is cosmetic rather than practical. Non-blocking. 2. **Discovered scope -- other pages with the same `100vh` pattern:** The repo has 6 additional `100vh` centering containers that will exhibit the same mobile bug: - `src/app.css` line 3372: `.auth-page` (flex centered, same pattern) - `src/routes/(app)/reset-password/+page.svelte` line 155: `.reset-page` - `src/routes/(app)/checkout/cancel/+page.svelte` line 32: `.cancel-page` - `src/routes/(app)/checkout/success/+page.svelte` line 17: `.success-page` - `src/routes/(app)/checkout/+page.svelte` line 258: `.checkout-page` (padding-only, less impacted) - `src/routes/(app)/+layout.svelte` line 83: inline style on loading spinner div This is **not a blocker for this PR** -- the PR correctly fixes the two pages specified in issue #154. But these other pages should be tracked as discovered scope in a new Forgejo issue. ### SOP COMPLIANCE - [x] Branch named after issue: `154-fix-mobile-signin-centering` references issue #154 - [x] PR body follows template: Summary, Changes, Test Plan, Related all present - [x] Related references issue: `Closes #154` - [ ] Related references plan slug: N/A (bug fix, no plan) - [x] No secrets committed - [x] No scope creep -- changes are tightly scoped to the two pages in the issue - [x] Commit message is descriptive (`fix: use 100dvh for mobile sign-in page centering`) ### PROCESS OBSERVATIONS - **Change failure risk:** Very low. CSS-only change, two properties, well-understood fix for a well-documented browser behavior difference. - **Discovered scope:** The 6 other `100vh` flex-centered containers should get their own issue to avoid the same bug report recurring for different pages. - **No test plan gap:** CSS viewport unit changes are not testable via unit tests. The PR body correctly specifies manual verification steps (390px viewport, real phone). ### VERDICT: APPROVED