From b617308c9c0ce7bf83a10de255454db929ae62d0 Mon Sep 17 00:00:00 2001 From: Jim Bethancourt Date: Thu, 17 Sep 2026 05:56:43 -0500 Subject: [PATCH 1/3] Initial layout cleanup --- AGENTS.md | 2 +- app/about/page.jsx | 2 +- app/api/page.jsx | 2 +- app/documentation/page.jsx | 2 +- app/examples/page.jsx | 2 +- app/faq/page.jsx | 2 +- app/feedback/page.jsx | 2 +- app/getting-started/page.jsx | 2 +- app/globals.css | 36 +++++++++ app/privacy-policy/page.jsx | 2 +- app/terms-of-service/page.jsx | 2 +- components/landing.jsx | 4 +- components/repo-submission-form.jsx | 2 +- playwright.config.js | 3 + tests/e2e/layout.spec.js | 90 ++++++++++++++++++++++ tests/integration/landing-page.test.jsx | 5 ++ tests/integration/static-pages.test.jsx | 9 +++ tests/integration/submission-form.test.jsx | 9 +++ tests/unit/layout-styles.test.js | 58 ++++++++++++++ 19 files changed, 223 insertions(+), 13 deletions(-) create mode 100644 tests/e2e/layout.spec.js create mode 100644 tests/unit/layout-styles.test.js diff --git a/AGENTS.md b/AGENTS.md index b6e39c1..630e7b2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -111,4 +111,4 @@ tests/ # unit/ (Bun), integration/ (Bun + RTL/jsdom), e2e/ ## Current Test Count -~271 unit/integration + 81 E2E (three browsers + basePath leg). +~294 unit/integration + 112 E2E (three browsers + basePath leg). diff --git a/app/about/page.jsx b/app/about/page.jsx index fd54ede..0d85e57 100644 --- a/app/about/page.jsx +++ b/app/about/page.jsx @@ -1,6 +1,6 @@ export default function AboutPage() { return ( -
+

About RefactorFirst

RefactorFirst is a static analysis tool that identifies which classes in your Java codebase you should refactor first, ranked by cost-benefit. It is based on the paper diff --git a/app/api/page.jsx b/app/api/page.jsx index 6e56e83..a570de2 100644 --- a/app/api/page.jsx +++ b/app/api/page.jsx @@ -1,6 +1,6 @@ export default function ApiPage() { return ( -

+

API for Tool Integrations

Fetching reports programmatically

diff --git a/app/documentation/page.jsx b/app/documentation/page.jsx index e0321e9..4290304 100644 --- a/app/documentation/page.jsx +++ b/app/documentation/page.jsx @@ -2,7 +2,7 @@ import Link from 'next/link'; export default function DocumentationPage() { return ( -
+

Documentation

Generating Reports

diff --git a/app/examples/page.jsx b/app/examples/page.jsx index 638e48f..5b621d4 100644 --- a/app/examples/page.jsx +++ b/app/examples/page.jsx @@ -2,7 +2,7 @@ import Link from 'next/link'; export default function ExamplesPage() { return ( -
+

Example Reports

See RefactorFirst reports for real projects. Each link opens the live report rendered from that repository.

diff --git a/app/faq/page.jsx b/app/faq/page.jsx index fba2ac7..69b1a44 100644 --- a/app/faq/page.jsx +++ b/app/faq/page.jsx @@ -2,7 +2,7 @@ import Link from 'next/link'; export default function FaqPage() { return ( -
+

Frequently Asked Questions

What do the different priority colors mean?

diff --git a/app/feedback/page.jsx b/app/feedback/page.jsx index 08d3b8d..36fa7fb 100644 --- a/app/feedback/page.jsx +++ b/app/feedback/page.jsx @@ -1,6 +1,6 @@ export default function FeedbackPage() { return ( -
+

Feedback

We welcome your feedback! The best place to share ideas, report bugs or request features is the RefactorFirst project on GitHub.

diff --git a/app/getting-started/page.jsx b/app/getting-started/page.jsx index 31452a5..a91a077 100644 --- a/app/getting-started/page.jsx +++ b/app/getting-started/page.jsx @@ -3,7 +3,7 @@ import WorkflowSample from '../../components/workflow-sample'; export default function GettingStartedPage() { return ( -
+

Getting Started

1. Configure the RefactorFirst Maven plugin

diff --git a/app/globals.css b/app/globals.css index 3d821fe..3b0664f 100644 --- a/app/globals.css +++ b/app/globals.css @@ -53,6 +53,30 @@ main:focus { outline: none; } +/* mvp.css makes every `section` a wrapping flex row with no gap and center + justification (`section { display: flex; flex-wrap: wrap; justify-content: + var(--justify-important) }`). It targets the mvp `section > aside` card + pattern, but our pages use sections as plain prose containers — so headings, + paragraphs, lists and code blocks were laid out side by side as flex items + and smashed together. mvp.css is loaded after this bundle, so the reset must + win on specificity: `main section` (0-0-2) beats the bare `section` + selector (0-0-1) regardless of cascade order. */ +main section { + display: block; +} + +/* Prose pages: roughly 75ch measure, centered in the wide main column. */ +.content-page { + max-width: 48rem; + margin: 0 auto; +} + +/* Landing sections (CTA, featured repos) that keep centered text now that + sections no longer flex-center their children. */ +.section-center { + text-align: center; +} + .hero { text-align: center; padding: 2.5rem 1rem; @@ -281,6 +305,17 @@ pre { color: #fff; } +/* ---- User listing (/user) ---- */ +.repo-listing h1, +.repo-listing .listing-subtitle { + text-align: center; +} + +.listing-subtitle { + margin-top: 0.25rem; + color: var(--muted-color); +} + /* ---- Forms / add-repo ---- */ .user-info { display: flex; @@ -297,6 +332,7 @@ pre { display: grid; gap: 0.5rem; max-width: 420px; + margin: 0 auto; } .form-status.error { diff --git a/app/privacy-policy/page.jsx b/app/privacy-policy/page.jsx index 36fe04a..7ae0f45 100644 --- a/app/privacy-policy/page.jsx +++ b/app/privacy-policy/page.jsx @@ -1,6 +1,6 @@ export default function PrivacyPolicyPage() { return ( -
+

Privacy Policy

Data Collected

diff --git a/app/terms-of-service/page.jsx b/app/terms-of-service/page.jsx index f9f5e28..57ba59a 100644 --- a/app/terms-of-service/page.jsx +++ b/app/terms-of-service/page.jsx @@ -1,6 +1,6 @@ export default function TermsOfServicePage() { return ( -
+

Terms of Service

Acceptable use

diff --git a/components/landing.jsx b/components/landing.jsx index 27b8602..59a16f8 100644 --- a/components/landing.jsx +++ b/components/landing.jsx @@ -15,10 +15,10 @@ export default function Landing({ repositories }) {

Know which parts of your codebase to refactor first. Search for a repository to see its report.

-
+
Add My Repo
-
+

Featured Repositories

    diff --git a/components/repo-submission-form.jsx b/components/repo-submission-form.jsx index e5c1201..734190f 100644 --- a/components/repo-submission-form.jsx +++ b/components/repo-submission-form.jsx @@ -62,7 +62,7 @@ export default function RepoSubmissionForm({ onExternalRedirect }) { const label = platformLabel(environment); return ( -
    +

    Add Your Repository

    Only repositories with a .refactorfirst/refactor-first.json file diff --git a/playwright.config.js b/playwright.config.js index 19f0f61..d2f0c77 100644 --- a/playwright.config.js +++ b/playwright.config.js @@ -33,5 +33,8 @@ export default defineConfig({ command: 'bun run build && python3 scripts/serve-out.py', port: 8003, timeout: 300 * 1000, + // Locally (bun run build + serve-out.py already running) reuse the + // existing server; CI always gets a fresh build. + reuseExistingServer: !process.env.CI, }, }); \ No newline at end of file diff --git a/tests/e2e/layout.spec.js b/tests/e2e/layout.spec.js new file mode 100644 index 0000000..b5b14bf --- /dev/null +++ b/tests/e2e/layout.spec.js @@ -0,0 +1,90 @@ +// Layout cleanup (plans/layout-cleanup-plan.md): mvp.css makes every +// `section` a wrapping flex row with no gap, which smashed prose children +// together (e.g. on /getting-started the

    and the first

    shared one +// line). These tests pin the corrected behavior: block flow — each child of a +// content section starts strictly below the previous child — plus a bounded, +// centered content column. +import { test, expect } from '@playwright/test'; + +// Returns, for each
    matching `selector`, the client rects of its +// direct children (in DOM order). +function sectionChildRects(selector) { + return Array.from(document.querySelectorAll(selector)).map(section => + Array.from(section.children).map(el => el.getBoundingClientRect()) + ); +} + +// Every child must start strictly below the previous child's bottom +// (2px tolerance for sub-pixel rounding). +function expectStacked(rects, context) { + const failures = []; + for (let i = 1; i < rects.length; i++) { + if (rects[i].top < rects[i - 1].bottom - 2) { + failures.push(`child ${i - 1} (bottom ${rects[i - 1].bottom}) and child ${i} (top ${rects[i].top}) share a row`); + } + } + expect(failures, `${context}: children must stack vertically`).toEqual([]); +} + +const CONTENT_PAGES = ['/about/', '/getting-started/', '/add-repo/']; + +test.describe('content page layout', () => { + test.use({ viewport: { width: 1280, height: 900 } }); + + for (const path of CONTENT_PAGES) { + test(`${path} stacks section children vertically`, async ({ page }) => { + await page.goto(path); + const sections = await page.evaluate(sectionChildRects, 'main section'); + expect(sections.length).toBeGreaterThan(0); + for (const rects of sections) expectStacked(rects, path); + }); + + test(`${path} keeps prose in a bounded, centered column`, async ({ page }) => { + await page.goto(path); + const box = await page.locator('main section.content-page').boundingBox(); + expect(box).toBeTruthy(); + expect(box.width).toBeLessThanOrEqual(800); + const left = box.x; + const right = 1280 - box.x - box.width; + expect(Math.abs(left - right)).toBeLessThanOrEqual(8); + }); + } + + test('getting-started regression: title and first heading do not share a row', async ({ page }) => { + await page.goto('/getting-started/'); + const [h1, firstH2] = await Promise.all([ + page.locator('main section h1').boundingBox(), + page.locator('main section h2').first().boundingBox(), + ]); + expect(h1).toBeTruthy(); + expect(firstH2).toBeTruthy(); + expect(firstH2.y).toBeGreaterThanOrEqual(h1.y + h1.height - 2); + }); +}); + +test.describe('landing page layout', () => { + test.use({ viewport: { width: 1280, height: 900 } }); + + test('hero, call-to-action and featured sections stack vertically', async ({ page }) => { + await page.goto('/'); + await page.waitForLoadState('networkidle'); + const rects = await page.evaluate(() => + Array.from(document.querySelectorAll('main > section, main > .hero-search')).map(el => + el.getBoundingClientRect() + ) + ); + expect(rects.length).toBeGreaterThanOrEqual(3); + expectStacked(rects, 'landing'); + }); + + test('featured heading sits above the repository grid, not beside it', async ({ page }) => { + await page.goto('/'); + const [heading, grid] = await Promise.all([ + page.locator('main section h2').boundingBox(), + page.locator('.featured-repos ul').boundingBox(), + ]); + expect(heading).toBeTruthy(); + expect(grid).toBeTruthy(); + expect(grid.y).toBeGreaterThanOrEqual(heading.y + heading.height - 2); + }); +}); diff --git a/tests/integration/landing-page.test.jsx b/tests/integration/landing-page.test.jsx index 38908e4..26d37e8 100644 --- a/tests/integration/landing-page.test.jsx +++ b/tests/integration/landing-page.test.jsx @@ -35,12 +35,17 @@ describe('Landing page', () => { const cta = screen.getByRole('link', { name: 'Add My Repo' }); expect(cta.getAttribute('href')).toBe('/add-repo'); expect(cta.className).toBe('cta'); + // Layout cleanup: the CTA section is centered (block-flowed prose layout). + expect(cta.closest('section').className).toContain('section-center'); }); test('lists the first 6 repositories as featured links', () => { render(_jsx(Landing, { repositories: REPOS })); const featured = document.querySelector('.featured-repos'); expect(featured).toBeTruthy(); + // Layout cleanup: the featured section is centered (block-flowed prose + // layout, see plans/layout-cleanup-plan.md). + expect(featured.closest('section').className).toContain('section-center'); const links = [...featured.querySelectorAll('a')]; expect(links.map(a => a.textContent)).toEqual([ 'apache/tomcat', diff --git a/tests/integration/static-pages.test.jsx b/tests/integration/static-pages.test.jsx index b4548af..8641a02 100644 --- a/tests/integration/static-pages.test.jsx +++ b/tests/integration/static-pages.test.jsx @@ -32,6 +32,15 @@ describe('static content pages', () => { const html = renderToStaticMarkup(_jsx(Page, {})); expect(html).toContain(`

    ${heading}

    `); }); + + // Layout cleanup: prose pages carry the .content-page class so the CSS + // gives them a bounded, centered, block-flowed column (see + // plans/layout-cleanup-plan.md — mvp.css would otherwise make the + // section a wrapping flex row that smashes the children together). + test(`/${name} uses the content-page layout class`, () => { + const html = renderToStaticMarkup(_jsx(Page, {})); + expect(html).toMatch(/^
    /); + }); } test('getting-started carries the workflow-sample slot', () => { diff --git a/tests/integration/submission-form.test.jsx b/tests/integration/submission-form.test.jsx index f971401..29f7266 100644 --- a/tests/integration/submission-form.test.jsx +++ b/tests/integration/submission-form.test.jsx @@ -67,6 +67,15 @@ describe('RepoSubmissionForm', () => { expect(container.querySelector('.form-status')?.getAttribute('aria-live')).toBe('polite'); }); + // Layout cleanup: the add-repo section uses the bounded prose column + // (plans/layout-cleanup-plan.md). + test('uses the content-page layout class on its section', () => { + const { container } = renderForm(); + const section = container.querySelector('section'); + expect(section.className).toContain('add-repo-page'); + expect(section.className).toContain('content-page'); + }); + test('shows inline validation errors for empty fields', async () => { const { container } = renderForm(); submit(container, {}); diff --git a/tests/unit/layout-styles.test.js b/tests/unit/layout-styles.test.js new file mode 100644 index 0000000..10e1992 --- /dev/null +++ b/tests/unit/layout-styles.test.js @@ -0,0 +1,58 @@ +// Layout cleanup: protect the mvp.css section-flex reset and the readability +// rules (plans/layout-cleanup-plan.md). mvp.css makes every `section` a +// wrapping flex row, which smashes prose children together; globals.css must +// neutralize it and give prose pages a readable, centered column. + +import { describe, test, expect } from 'bun:test'; +import { readFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import { dirname, join } from 'node:path'; + +const css = readFileSync( + join(dirname(fileURLToPath(import.meta.url)), '../../app/globals.css'), + 'utf-8' +); + +// Naive-but-sufficient block parser: strips comments, then maps each selector +// to its declaration body (e.g. "main section" -> "display: block; ..."). +function ruleMap(source) { + const noComments = source.replace(/\/\*[\s\S]*?\*\//g, ''); + const rules = new Map(); + const ruleRe = /([^{}]+)\{([^{}]*)\}/g; + let match; + while ((match = ruleRe.exec(noComments)) !== null) { + for (const selector of match[1].split(',')) { + rules.set(selector.trim(), match[2]); + } + } + return rules; +} + +const rules = ruleMap(css); + +describe('layout styles (mvp.css section-flex reset)', () => { + test('main sections render as a normal block container, not a flex row', () => { + const body = rules.get('main section'); + expect(body).toBeTruthy(); + expect(body).toMatch(/display:\s*block\s*;/); + }); + + test('prose pages get a bounded, centered content column via .content-page', () => { + const body = rules.get('.content-page'); + expect(body).toBeTruthy(); + expect(body).toMatch(/max-width:\s*\d+/); + expect(body).toMatch(/margin:\s*0\s+auto\s*;/); + }); + + test('landing CTA / featured sections can be centered via .section-center', () => { + const body = rules.get('.section-center'); + expect(body).toBeTruthy(); + expect(body).toMatch(/text-align:\s*center\s*;/); + }); + + test('the add-repo form is centered on its now block-flowed page', () => { + const body = rules.get('#repo-form'); + expect(body).toBeTruthy(); + expect(body).toMatch(/margin:\s*0\s+auto\s*;/); + }); +}); From 789b95feab5c34435ad54a2e99cbf4ab1d3e17bd Mon Sep 17 00:00:00 2001 From: Jim Bethancourt Date: Thu, 17 Sep 2026 07:37:17 -0500 Subject: [PATCH 2/3] Centered hero search on landing page --- app/globals.css | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/app/globals.css b/app/globals.css index 3b0664f..043829a 100644 --- a/app/globals.css +++ b/app/globals.css @@ -249,11 +249,22 @@ pre { .hero-search { position: relative; - max-width: 480px; + /* Exactly half the rendered width of the top menu (#top-menu is capped by + mvp.css at --width-content = 1080px, full width below that) and centered + under it. */ + width: min(540px, 50vw); margin: 1rem auto; text-align: left; } +.hero-search input { + width: 100%; + box-sizing: border-box; + padding: 0.35rem 0.6rem; + border: 1px solid #c6d2dc; + border-radius: var(--radius); +} + /* ---- Repository grid ---- */ .repo-grid, .featured-repos ul { From 508cce83a6c769f401a46d9ed3bc7aaab53b1a20 Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Thu, 17 Sep 2026 12:51:17 +0000 Subject: [PATCH 3/3] Require PLAYWRIGHT_REUSE_SERVER=true to reuse an existing Playwright server --- playwright.config.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/playwright.config.js b/playwright.config.js index d2f0c77..e9b165a 100644 --- a/playwright.config.js +++ b/playwright.config.js @@ -33,8 +33,7 @@ export default defineConfig({ command: 'bun run build && python3 scripts/serve-out.py', port: 8003, timeout: 300 * 1000, - // Locally (bun run build + serve-out.py already running) reuse the - // existing server; CI always gets a fresh build. - reuseExistingServer: !process.env.CI, + // Reuse a locally started server only when explicitly requested. + reuseExistingServer: process.env.PLAYWRIGHT_REUSE_SERVER === 'true', }, -}); \ No newline at end of file +});