Repository navigation
fix(frontend): make landing page usable at mobile widths - #1865
LucasSantana-Dev merged 4 commits into
Conversation
|
Warning Review limit reached
Next review available in: 20 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
All reported issues were addressed across 1 file
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Auto-approved: CSS layout adjustments to eliminate horizontal overflow on mobile widths: stacking command rows, truncating nav labels, and tightening padding. A bounded, clearly beneficial fix that Cubic has already reviewed for correctness.
Re-trigger cubic
6b5ffc0 to
15d57ea
Compare
Kimi review (
|
There was a problem hiding this comment.
This is a good pass. You went after the actual cause rather than hiding it with overflow-hidden: min-w-0 at each flex boundary, truncate on the repo name paired with shrink-0 on the license pill so the pill doesn't get crushed, and restacking the command list to flex-col below sm. Dropping the headline to clamp(2rem, 8vw, 4.4rem) with break-words is the right move for narrow viewports. The accessibility handling is also correct: aria-hidden on both the short and long label spans with aria-label on the anchor means the name comes from the label and the responsive text isn't announced twice.
Two things before merge.
1. Conflicts with #1874, and yours drops flex-1. #1874 fixes the same RepoCard <code>:
- #1874:
min-w-0 flex-1 truncate text-lucky-text-body - yours:
min-w-0 truncate text-lucky-text-body
Without flex-1 the <code> doesn't claim the remaining width inside the justify-between row, so it truncates earlier than intended. Please keep flex-1 when you rebase. I'll sequence the two so we don't lose it.
2. min-[400px] is now load-bearing but undiscoverable. It appears four times across TopNav as an arbitrary value. Nobody scanning the Tailwind config will know 400px is meaningful, so the next person adding a nav item won't match it. Could you promote it to a named screen (xs or similar) in the theme, so it has one definition? Not a blocker, but it'll rot otherwise.
Nit: worth a quick check that sm:contents on the new wrapper does what you want, since display: contents removes the box from layout entirely and it's carrying the sm:order-last badge placement. Fine in current browsers, just the kind of thing that's easy to break later.
On the red checks: not yours. Security was an unpassable repo-wide gate, kimi-review fails on every PR because an API key isn't set, and the npm ci errors come from a stale lockfile on main. Fixed in #1876 (plus #1877 for the key). Rebase after that lands.
|
Heads-up: the CI blockers I mentioned are fixed on What that clears:
Please rebase onto The review feedback above is separate and still stands. |
5461ca4 to
36e5d3e
Compare
|
Addressed in 73eeb97 (squashed, rebased onto main).
|
36e5d3e to
73eeb97
Compare
LucasSantana-Dev
left a comment
There was a problem hiding this comment.
Review: approve
Layout-only change that does what it claims, keeps a11y intact where visible text is removed, and all 22 existing Landing tests still apply (ran them on base: 22/22 green).
P3 (nit): Landing.tsx:169-189 — the xs-toggle span pair (<span className='xs:hidden'>add</span> / <span className='hidden xs:inline'>add to discord</span>) is duplicated verbatim across the enabled <a> and disabled <button> CTA branches, so a future label tweak needs two edits. A tiny CtaLabel fragment would dedupe it. (The two-branch duplication is pre-existing, so optional.)
What's good: a11y handled correctly at the exact points where visible text disappears — the GitHub nav link gains aria-label='GitHub' when it becomes icon-only below 400px, and the CTA spans are aria-hidden with aria-label='Add to Discord' on the parent, so screen readers get one clean name. The sm:contents + sm:order-last pattern in CommandList is a clean way to get badge-next-to-name on mobile and badge-last on sm+ without restructuring the DOM. --breakpoint-xs: 400px via Tailwind v4 @theme is idiomatic and purely additive (zero existing xs: usages).
min-w-0 at flex boundaries, truncate on the repo name, flex-col command list below sm, and a fluid headline. Keep flex-1 on the clone code row so truncate claims remaining width. Name the 400px nav threshold as an xs breakpoint.
73eeb97 to
e30848f
Compare
|
Rebased onto main and kept
|
## Summary Two structural failures block every fork PR's required checks (seen on #1863, #1864, #1865, #1866, #1867, #1674 after their CI was approved): - **SonarCloud Scan (required) hard-fails on forks**: fork PRs get no secrets, so `SONAR_TOKEN` is never present and the token-policy step exits 1. Now the sonar job is skipped for fork PRs (a skipped required check counts as passing). Same pattern deploy-staging already uses. - **danger 403s on forks**: `review-tools.yml` ran on `pull_request`, where the fork token is forced read-only and the comment POST fails with 403. Switched to `pull_request_target`; the reusable workflow checks out and executes base-repo code only (documented in the file header, same safety rule as the other target workflows). ## Test plan - [x] actionlint clean on both files - [ ] Next push to an external contributor PR: SonarCloud Scan shows skipped, danger posts its comment After merge I will update the seven open contributor branches to main so they pick this up. <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Unblocks fork PRs by fixing CI gates for SonarCloud and `danger`. Fork PRs now pass required checks without secrets and get review comments. - Bug Fixes - Skip SonarCloud Scan on fork PRs to avoid failing when `SONAR_TOKEN` is unavailable (skipped required check counts as passing). - Run review tools on `pull_request_target` so `danger` can comment on forks; workflow executes base-repo code only. <sup>Written for commit 316f567. Summary will update on new commits.</sup> <a href="https://cubic.dev/pr/LucasSantana-Dev/Lucky/pull/1898?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
|
A note from the maintainer side: sorry this PR waited as long as it did for a proper review, and sorry for the rounds of branch updates and re-running checks today. The churn was on our side, not yours. Your PRs exposed real gaps in how this repo handled external contributions: CI runs sat in a silent approval queue, some gates could never pass on fork PRs (SonarCloud, danger), and the team had no notification when external PRs arrived. Those are all fixed as of today:
Your branch is up to date and the full suite is green. Thanks for the patience and for the contribution. External contributors are very welcome here. |
🤖 I have created a release *beep* *boop* --- <details><summary>2.38.0</summary> ## [2.38.0](v2.37.3...v2.38.0) (2026-07-27) ### Features * **bot:** add /ticket-setup for support category and agent role ([#1863](#1863)) ([3f4af39](3f4af39)) * **frontend:** per-action loading and connection gating on music controls ([#1866](#1866)) ([2dda60f](2dda60f)) * **frontend:** show stale progress when music SSE lags ([#1867](#1867)) ([4952e73](4952e73)) * **music:** surface recommendationReason in nowplaying and queue ([#1864](#1864)) ([960fd62](960fd62)) * **ops:** blue/green zero-downtime deploys — Phase 1 web tier ([#1786](#1786)) ([f5f7597](f5f7597)) ### Bug Fixes * **docker:** make compose stack boot from a fresh .env ([#1674](#1674)) ([babe0ef](babe0ef)) * **docker:** treat an empty db password as missing in compose guards ([#1881](#1881)) ([718c0ad](718c0ad)) * **frontend:** make landing page usable at mobile widths ([#1865](#1865)) ([6190350](6190350)) * **frontend:** stop hero grid columns overflowing on narrow viewports ([#1874](#1874)) ([ce5cea0](ce5cea0)) * **invite:** add /invite where cloudflare pages reads it ([#1895](#1895)) ([0528f66](0528f66)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Description
Operator report: the public landing (https://lucky.lucassantana.tech) is not usable on phone widths.
Static review of
Landing.tsxfound a few concrete overflow sources even though Tailwind breakpoints already exist:w-[120px]command column, so name + description + badge overflowed around 360px.2.6rem) and a few cards lackedmin-w-0/ wrap, so long strings could spill.Fix
sm; only apply fixed command width fromsmup.min-w-0, truncate where needed.Verification
Checklist
Destructive / irreversible interaction (Tier A)
Not applicable.
Feature-removal sweep
Not applicable.
Fixes #1825
Summary by cubic
Make the public landing page usable on phone widths by removing horizontal overflow and fitting the nav, hero, and command list. Fixes #1825.
xs(400px) breakpoint in@theme; GitHub is icon-only and invite shows “add” atxs;aria-labels added; tighter gaps;min-w-0+whitespace-nowrapprevent overflow and are retained after merge.break-wordsfor long strings.sm; fixed command width applies fromsm+; description usesmin-w-0/flex-1with truncation onsm+; badge sits with the command on mobile.min-w-0at flex boundaries and truncation where needed; repo name truncates, license pill isshrink-0; clone line keepsflex-1+min-w-0; tighter mobile padding withbreak-words.Written for commit 1c6505b. Summary will update on new commits.