Skip to content

fix(website): unstretch See all case studies button#11854

Merged
Yourz merged 3 commits intomainfrom
glary/fix-case-study-button-stretched
May 4, 2026
Merged

fix(website): unstretch See all case studies button#11854
Yourz merged 3 commits intomainfrom
glary/fix-case-study-button-stretched

Conversation

@christian-byrne
Copy link
Copy Markdown
Contributor

@christian-byrne christian-byrne commented May 3, 2026

PR Created by the Glary-Bot Agent


Summary

The "See all case studies" button on the homepage CaseStudySpotlightSection was rendering oddly stretched because it had class="flex-1 text-center" while being the sole child of a flex-row container — it expanded to fill the entire content column (~592px) instead of sizing to its label.

This drops flex-1/text-center and adds items-start to the wrapper so the button sizes to its content and is left-aligned, matching the proportions of every other outline BrandButton on the site (Hero, UseCase, customer detail, etc.).

Changes

  • apps/website/src/components/home/CaseStudySpotlightSection.vue: remove flex-1 text-center from the BrandButton and align the row's items to the start.

BrandButton already centers its label internally via inline-flex … justify-center, so dropping text-center is a no-op visually.

Before / After

  • Desktop before: button width = 592px (stretched across the column)
  • Desktop after: button width = 223px (natural)
  • Mobile: 1-column layout, now consistently left-aligned

Review Focus

Whether the fix should also live on the BrandButton component itself (e.g. a global max-width) instead of at the call site. I went with the instance-level fix because every other CTA in the website intentionally uses bare BrandButton and lets the content size it; only this one had flex-1. A blanket max-width would risk changing Hero/MobileMenu buttons that explicitly opt into w-full lg:w-auto lg:min-w-60.

Screenshots

Before: button stretched across the full content column

After: button sized to content, left-aligned

After: mobile view, left-aligned natural width

┆Issue is synchronized with this Notion page by Unito

The button used flex-1 inside a flex-row container, causing it to stretch to fill the entire content column. Drop flex-1/text-center and align the row's items to the start so the button sizes naturally and matches the proportions of other outline buttons across the site.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 3, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e390a298-a49d-4ba4-a7c6-6bce537d768b

📥 Commits

Reviewing files that changed from the base of the PR and between 06831c5 and c249319.

📒 Files selected for processing (2)
  • apps/website/e2e/homepage.spec.ts
  • apps/website/src/components/home/CaseStudySpotlightSection.vue
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/website/src/components/home/CaseStudySpotlightSection.vue

📝 Walkthrough

Walkthrough

Right-side content container in the CaseStudySpotlight component receives a test id; CTA row wrapper spacing and alignment are adjusted; the BrandButton's flex-1 text-center classes are removed. Two Playwright tests were added to assert CTA sizing and mobile vertical spacing.

Changes

CaseStudySpotlight UI + Tests

Layer / File(s) Summary
Markup / Identifier
apps/website/src/components/home/CaseStudySpotlightSection.vue
Right content wrapper now includes data-testid="case-study-content" and preserves class="flex flex-col justify-between p-6 lg:flex-1".
Layout / Styling
apps/website/src/components/home/CaseStudySpotlightSection.vue
CTA row wrapper updated to class="mt-8 flex flex-col items-start gap-3 sm:flex-row lg:mt-0"; BrandButton no longer receives class="flex-1 text-center".
End-to-end Tests
apps/website/e2e/homepage.spec.ts
Added two Playwright assertions: (1) CTA width is less than 70% of the content column; (2) mobile-tagged test asserts at least 24px vertical gap between subheading and CTA by comparing bounding boxes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I hopped to the spotlight with padding neat,
A tidy CTA no longer stretched to its feet,
Aligned to the start, with space to breathe,
Tests check the gap — all tucked in beneath. 🐰✨


Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

  • Ignore (reviewers only)

❌ Failed checks (1 error)

Check name Status Explanation Resolution
End-To-End Regression Coverage For Fixes ❌ Error PR title contains 'fix' (bug-fix language) and regression tests were added to apps/website/e2e/ instead of the required browser_tests/ directory. Move regression tests under browser_tests/ directory or add PR description explaining why apps/website/e2e/ location is appropriate for this project's testing structure.
✅ Passed checks (6 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: removing the flex-1 class that was causing the button to stretch, making it a proper fix for the button's layout issue.
Description check ✅ Passed The description comprehensively covers all template sections: a clear summary, detailed changes with before/after measurements, thoughtful review focus explaining design decisions, and supporting screenshots demonstrating the fix.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Adr Compliance For Entity/Litegraph Changes ✅ Passed PR changes are limited to apps/website/ directory (Vue component and E2E test), not to src/lib/litegraph/, src/ecs/, or graph entity files.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch glary/fix-case-study-button-stretched

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get your free trial and get 200 agent minutes per Slack user (a $50 value).


Review rate limit: 1/5 review remaining, refill in 36 minutes and 3 seconds.

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 3, 2026

🌐 Website E2E

Tip

All tests passed.

Status ✅ Passed
Report View Report

🔗 Website Preview

Website Preview: https://comfy-website-preview-pr-11854.vercel.app

This commit: https://website-frontend-ksndyty1u-comfyui.vercel.app

Last updated: 2026-05-03T06:22:42Z for c249319

@glary-bot
Copy link
Copy Markdown

glary-bot Bot commented May 3, 2026

Re CodeRabbit's "End-To-End Regression Coverage For Fixes" check:

This is a 4-line, declarative CSS change (drop flex-1 text-center, add items-start) to a static marketing component with no behavior. A Playwright spec asserting "this <a> does not have width: 100%" would be a tautological change-detector test against Tailwind class output — exactly the anti-pattern called out in AGENTS.md ("Do not write tests that are dependent on non-behavioral features like utility classes or styles" / "Do not write change detector tests").

Visual verification was done manually with Playwright at desktop (1440px: 592px → 223px) and mobile (390px) — screenshots are in the PR description. Skipping E2E coverage for this fix.

Glary-Bot added 2 commits May 3, 2026 06:10
…n test

Mobile layout had no min gap between subheading and the CTA because the column relied on flex justify-between, which collapses when the column has no fixed height. Add mt-8 so the CTA always sits ~32px below the subheading on mobile (desktop is unchanged — justify-between still expands to fill the video-aligned column height).

Add an E2E test that fails on the original regression: it asserts the CTA width is well below the content column width, which catches both the original flex-1 stretching and any future change that re-introduces full-width behavior.
Address review feedback:

- Scope mt-8 to mobile only (lg:mt-0) since the desktop column already gets adequate spacing from flex justify-between against the video height. The previous unscoped mt-8 was a no-op on desktop but mismatched intent.
- Add stable data-testid='case-study-content' selector instead of walking up the DOM with .locator('..').locator('..').
- Split the regression test in two: keep the desktop width assertion, and add a mobile-tagged sibling that verifies vertical breathing room (>=24px) above the CTA on Pixel 5 — which is where the spacing bug actually manifests.
@christian-byrne christian-byrne marked this pull request as ready for review May 3, 2026 06:36
@christian-byrne christian-byrne requested a review from a team May 3, 2026 06:36
@dosubot dosubot Bot added the size:S This PR changes 10-29 lines, ignoring generated files. label May 3, 2026
@christian-byrne christian-byrne requested review from DrJKL and removed request for DrJKL May 3, 2026 06:42
@Yourz Yourz added this pull request to the merge queue May 4, 2026
@glary-bot
Copy link
Copy Markdown

glary-bot Bot commented May 4, 2026

Thanks for the approval @Yourz!

Re the failing "End-To-End Regression Coverage For Fixes" check (which keeps wanting tests under browser_tests/): this PR's tests are correctly placed in apps/website/e2e/. The apps/website Astro app is a separate workspace package with its own Playwright config (apps/website/playwright.config.tstestDir: './e2e'); the root browser_tests/ directory is exclusively for the main ComfyUI frontend. All other website tests (homepage.spec.ts, careers.spec.ts, cloud.spec.ts, etc.) live there for the same reason.

The check should be safe to ignore for apps/website/** changes.

Merged via the queue into main with commit 5523df1 May 4, 2026
49 checks passed
@Yourz Yourz deleted the glary/fix-case-study-button-stretched branch May 4, 2026 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants