fix(onboarding): review-feedback polish#7989
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThe onboarding inline input now uses Edit-oriented accessibility labels and filled styling, with updated end-to-end selectors. SDK code cards display accent badges with SDK logos, chip typography is adjusted, and the JavaScript logo is rendered inline. Connection-panel headings use updated font classes, obsolete language styling is removed, and onboarding top spacing is increased. Estimated code review effort: 3 (Moderate) | ~20 minutes 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 |
Docker builds report
|
|
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Failed testsfirefox › tests/onboarding-tests.pw.ts › Onboarding › New user connects via the single-page onboarding flow @oss Details
Failed testsfirefox › tests/onboarding-tests.pw.ts › Onboarding › New user connects via the single-page onboarding flow @oss Details
Failed testsfirefox › tests/onboarding-tests.pw.ts › Onboarding › New user connects via the single-page onboarding flow @oss Details
Failed testsfirefox › tests/onboarding-tests.pw.ts › Onboarding › New user connects via the single-page onboarding flow @oss Details
Failed testsfirefox › tests/onboarding-tests.pw.ts › Onboarding › New user connects via the single-page onboarding flow @oss Details
Failed testsfirefox › tests/onboarding-tests.pw.ts › Onboarding › New user connects via the single-page onboarding flow @oss Details
Failed testsfirefox › tests/onboarding-tests.pw.ts › Onboarding › New user connects via the single-page onboarding flow @oss Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Failed testsfirefox › tests/onboarding-tests.pw.ts › Onboarding › New user connects via the single-page onboarding flow @oss Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Failed testsfirefox › tests/onboarding-tests.pw.ts › Onboarding › New user connects via the single-page onboarding flow @oss Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Failed testsfirefox › tests/onboarding-tests.pw.ts › Onboarding › New user connects via the single-page onboarding flow @oss Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
|
Visual Regression19 screenshots compared. See report for details. |
23652a2 to
4550c7a
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c3593f93-b142-45e2-8fe8-b7804dd57432
📒 Files selected for processing (10)
frontend/common/code-help/init/init-react.jsfrontend/e2e/tests/onboarding-tests.pw.tsfrontend/web/components/pages/onboarding/InlineInput/InlineInput.scssfrontend/web/components/pages/onboarding/InlineInput/InlineInput.tsxfrontend/web/components/pages/onboarding/OnboardingConnectPanel/ConnectWithAiPanel.tsxfrontend/web/components/pages/onboarding/OnboardingConnectPanel/ConnectYourCodePanel.tsxfrontend/web/components/pages/onboarding/OnboardingConnectPanel/OnboardingConnectPanel.scssfrontend/web/components/pages/onboarding/OnboardingConnectPanel/SdkPicker.tsxfrontend/web/components/pages/onboarding/OnboardingConnectPanel/logos/javascript-logo.tsxfrontend/web/components/pages/onboarding/OnboardingFlow/OnboardingFlow.scss
| maxLength={maxLength} | ||
| aria-label={`${label} name`} | ||
| // Avoid a "name" label here: Safari reads it as a contact field. | ||
| aria-label={`Edit ${label.toLowerCase()}`} |
There was a problem hiding this comment.
"name" in the accessible label makes Safari offer contact autofill over the field, so it reads "Edit flag" now. The e2e label change is the same fix. We also gave the field a fill so it's more visible when editing.
| return ( | ||
| <> | ||
| <span className='text-default fw-semibold'> | ||
| <span className='text-default font-weight-semibold'> |
There was a problem hiding this comment.
Kyle's catch: fw-semibold has no effect in this codebase; font-weight-semibold is the working class, same weight as intended. Swapped on all the section headings.
bdfa0c3 to
96da811
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: db143a86-88dc-4826-bddd-8126f22309db
📒 Files selected for processing (6)
frontend/e2e/tests/onboarding-tests.pw.tsfrontend/web/components/pages/onboarding/InlineInput/InlineInput.scssfrontend/web/components/pages/onboarding/InlineInput/InlineInput.tsxfrontend/web/components/pages/onboarding/OnboardingConnectPanel/ConnectWithAiPanel.tsxfrontend/web/components/pages/onboarding/OnboardingConnectPanel/ConnectYourCodePanel.tsxfrontend/web/components/pages/onboarding/OnboardingConnectPanel/SdkPicker.tsx
Replace the plain text language label in the code card header with the same accent badge (logo + label) the SDK picker uses for the selected chip, in semibold. Reuses the Chip primitive rather than the bespoke label style. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The JS logo hole-punched the letters out of the yellow square, so they showed the chip behind them and washed out on a light chip. Draw it as the brand badge instead - solid black JS on a rounded yellow square - so it reads on both light and dark. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Apply fw-semibold to the selectable SDK chips (and the More/Less chip) so they match the selected-SDK badge in the code card. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Review nit: bump the flow's top padding (4px -> 24px) so the theme-toggle row isn't flush to the top edge. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Review feedback (Matt): the org/project fields only showed a fill on hover, so they read as plain text until clicked. Give them a resting fill (matching the flag pill) and a full-opacity pencil, deepening on hover for feedback. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Safari AutoFill read the "<thing> name" aria-label as a contact-name field and showed its person icon on the project field. Label the fields "Edit <thing>" instead: a clear accessible name Safari doesn't classify as a contact. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The flag rename step located the field via getByLabel('Flag name'); the field
is now labelled 'Edit flag' (Safari autofill fix), so point the selector there.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Review (Kyle): the chips/badge weren't rendering semibold. The project's working weight class is .font-weight-semibold (token + !important); Bootstrap's .fw-semibold has no effect here. Swap it across the connect panel. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Review (Kyle): the resting-fill fields felt cramped. Bump the radius to --radius-sm and the padding to 1px 8px, matching the flag pill. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Rounded bottom corners fought the action underline. Square the bottom so it sits flush; the flag pill (no underline) keeps its full radius. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Semibold read too heavy on the chips; use font-weight-medium for the SDK badge and picker chips. Section headings stay semibold. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
96da811 to
a08c5c0
Compare
Ports the onboarding code-card fix (was on #7989) to the shared embedded variant: overflow-x auto instead of a permanently reserved gutter. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
docs/if required so people know about the feature.Changes
Contributes to #7739
Polish for the onboarding quickstart flow, from review feedback on #7941 (Kyle, Wadii, Matt).
font-weight-medium).fw-semiboldwas a no-op; swapped to the project'sfont-weight-semibold).How did you test this code?
Manually against the staging preview, plus
lintandtypecheck. Open/getting-started(behind ffonboarding_quickstart_flow), toggle light/dark:E2E (
onboarding-tests.pw.ts) covers the rename flow via the new "Edit flag" label.Screenshots