Refresh product and solution page design#673
Open
riderx wants to merge 7 commits into
Open
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR restyles the CI/CD marketing page with comprehensive Tailwind class and markup updates across all sections (hero, problem/solution, platforms, how-it-works, urgency CTA, build visualizations, stats, benefits, features, expert service, pricing, and final CTA). Supporting changes include header logo spacing adjustment, pluginDocs initialization order fix, and CSS whitespace formatting. ChangesCI/CD Page Marketing Restyle
Supporting Minor Updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/pages/ci_cd.astro (1)
30-36:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd
aria-hiddento decorative inline SVGs.These icons are visual-only (paired with nearby text), so screen readers should ignore them to prevent duplicate announcements. Please add
aria-hidden="true"(and ideallyfocusable="false") consistently.Suggested pattern
-<svg class="mr-2 h-4 w-4" xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" stroke="currentColor"> +<svg class="mr-2 h-4 w-4" xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" stroke="currentColor" aria-hidden="true" focusable="false">As per coding guidelines "Decorative SVGs must have
aria-hidden="true"."Also applies to: 74-76, 111-117, 125-127, 137-139, 149-151, 161-163, 184-186, 194-196, 206-208, 218-220, 230-232, 253-257, 261-265, 355-357, 387-389, 452-454, 534-536, 542-544, 564-566, 578-585, 604-611, 641-647, 653-655, 676-684, 712-714, 754-761, 781-783, 796-805, 820-825, 838-847, 854-856, 869-876, 925-927, 955-957
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/pages/ci_cd.astro` around lines 30 - 36, The inline SVGs (e.g., the <svg class="mr-2 h-4 w-4"> element shown) are decorative and must be ignored by assistive tech; update each decorative inline <svg> (including those with class "mr-2 h-4 w-4") to include aria-hidden="true" and focusable="false" attributes (and ensure no role or title is present) across all occurrences listed in the review so screen readers don't announce them.
🧹 Nitpick comments (1)
apps/web/src/css/global.css (1)
307-309: ⚡ Quick winAvoid coupling logo styling to
alttext content.Line 307 targets
header img[alt$='logo']; this is fragile with localization/copy changes and can silently break logo theming. Prefer a stable class or data-attribute selector fromHeader.astro(for example.site-logo).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/css/global.css` around lines 307 - 309, The CSS rule targets header img[alt$='logo'] which is fragile; update the markup to add a stable class or data attribute on the logo element (e.g., add class "site-logo" or data-logo in Header.astro) and then replace the selector in global.css to target that stable symbol (e.g., .marketing-refresh header .site-logo) instead of relying on alt text; ensure you remove the alt$='logo' selector and keep the same filter styles applied to the new selector.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/css/global.css`:
- Around line 377-381: The current selector ".marketing-refresh
:where(a.bg-blue-600, a.bg-sky-600, a.bg-cyan-600, a.bg-emerald-600,
a.bg-green-600, a.bg-amber-600, a.bg-orange-600, a.bg-rose-600, a.bg-red-600)"
is removing box-shadow with "!important" and hiding Tailwind focus rings; to
fix, stop suppressing focus by either removing the "box-shadow: none !important"
declaration or scoping it so it does not apply when the element is focused
(e.g., change to ".marketing-refresh :where(...):not(:focus-visible) {
box-shadow: none !important }" and add an explicit ".marketing-refresh
:where(...):focus-visible { box-shadow: var(--tw-ring) !important; outline:
none; }" or, better, remove the "!important" and rely on Tailwind's focus
styles), ensuring keyboard focus rings remain visible while keeping the
non-focused styling.
---
Outside diff comments:
In `@apps/web/src/pages/ci_cd.astro`:
- Around line 30-36: The inline SVGs (e.g., the <svg class="mr-2 h-4 w-4">
element shown) are decorative and must be ignored by assistive tech; update each
decorative inline <svg> (including those with class "mr-2 h-4 w-4") to include
aria-hidden="true" and focusable="false" attributes (and ensure no role or title
is present) across all occurrences listed in the review so screen readers don't
announce them.
---
Nitpick comments:
In `@apps/web/src/css/global.css`:
- Around line 307-309: The CSS rule targets header img[alt$='logo'] which is
fragile; update the markup to add a stable class or data attribute on the logo
element (e.g., add class "site-logo" or data-logo in Header.astro) and then
replace the selector in global.css to target that stable symbol (e.g.,
.marketing-refresh header .site-logo) instead of relying on alt text; ensure you
remove the alt$='logo' selector and keep the same filter styles applied to the
new selector.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4a577b9d-7dc0-4417-99e6-17c1b8d5812f
📒 Files selected for processing (26)
apps/web/src/css/global.cssapps/web/src/layouts/Layout.astroapps/web/src/pages/app_mobile.astroapps/web/src/pages/ci_cd.astroapps/web/src/pages/index.astroapps/web/src/pages/integrations.astroapps/web/src/pages/live-update.astroapps/web/src/pages/native-build.astroapps/web/src/pages/plugins.astroapps/web/src/pages/solutions/agencies.astroapps/web/src/pages/solutions/beta-testing.astroapps/web/src/pages/solutions/cordova-to-capacitor-ai.astroapps/web/src/pages/solutions/cordova-to-capacitor.astroapps/web/src/pages/solutions/direct-updates.astroapps/web/src/pages/solutions/ecommerce.astroapps/web/src/pages/solutions/fintech.astroapps/web/src/pages/solutions/healthcare.astroapps/web/src/pages/solutions/ionic-enterprise-plugins.astroapps/web/src/pages/solutions/pr-preview.astroapps/web/src/pages/solutions/production-updates.astroapps/web/src/pages/solutions/qsr.astroapps/web/src/pages/solutions/solo-developers.astroapps/web/src/pages/solutions/startups.astroapps/web/src/pages/solutions/version-targeting.astroapps/web/src/pages/solutions/white-label.astroapps/web/src/services/pluginDocs.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/pages/ci_cd.astro (1)
50-57:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse an
h2for the hero callout title.After the page
h1, this jumps straight toh3, which skips a heading level in the document outline. Promote it toh2or restyle it as non-heading text.As per coding guidelines
**/*.{astro,html,jsx,tsx}: Headings must follow proper hierarchy (h1 > h2 > h3).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/pages/ci_cd.astro` around lines 50 - 57, The hero callout currently uses an <h3> for the title "What if builds happened <span class="text-gray-100">automatically on every commit?</span>" which skips a level after the page <h1>; change that <h3> to an <h2> (keeping the same classes: "mb-4 text-2xl font-bold text-white") so heading hierarchy is preserved and visual appearance remains the same, or alternatively replace the element with a non-heading tag if you intentionally want it excluded from the outline.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/css/global.css`:
- Around line 382-386: The current rule .marketing-refresh
:where(a.border-gray-600, a.border-white/30, a.bg-white/5) force-applies white
background and neutral text even on hover/focus, cancelling authored hover
styles; update it to only target the resting state (e.g., scope to the
non-interaction state with :not(:hover):not(:focus) or use a
:where(...):not(:hover):not(:focus) variant) or add matching themed hover/focus
rules for those same selectors (a.border-gray-600, a.border-white/30,
a.bg-white/5) that preserve or map to the intended hover styles (such as keeping
hover:bg-gray-700 behavior) so outlined CTA links regain their hover affordance.
- Around line 282-300: The .marketing-refresh rule flips dark background
utilities (e.g., .bg-gray-900, .bg-gray-800) to white but doesn't remap accent
text like .text-green-400, .text-blue-400, .text-orange-400, causing low
contrast (seen in terminal mockups). Fix by either excluding the specific
terminal/mockup container from the .marketing-refresh selector (so leave
.bg-gray-900/.bg-gray-800 unaffected) or add explicit color overrides inside
.marketing-refresh for the accent classes (e.g., map .marketing-refresh
.text-green-400/.text-blue-400/.text-orange-400 to darker hexs) and update the
CSS block that contains .marketing-refresh rules to include those accent
mappings.
In `@apps/web/src/pages/ci_cd.astro`:
- Around line 268-274: The logo images currently duplicate the adjacent visible
labels (e.g., the <img src="/icons/jenkins.svg"> with the <span>Jenkins</span>
and the <img src="/icons/circleci.svg"> with the <span>CircleCI</span>), causing
screen readers to announce the platform twice; make the images decorative by
setting their alt text to an empty string (alt="") so the accessible name comes
from the visible <span> labels, or alternatively remove the text spans and keep
meaningful alt text—pick one approach consistently for both Jenkins and CircleCI
and update the corresponding <img> and <span> elements accordingly.
---
Outside diff comments:
In `@apps/web/src/pages/ci_cd.astro`:
- Around line 50-57: The hero callout currently uses an <h3> for the title "What
if builds happened <span class="text-gray-100">automatically on every
commit?</span>" which skips a level after the page <h1>; change that <h3> to an
<h2> (keeping the same classes: "mb-4 text-2xl font-bold text-white") so heading
hierarchy is preserved and visual appearance remains the same, or alternatively
replace the element with a non-heading tag if you intentionally want it excluded
from the outline.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: da706b45-b01f-4524-a075-94861396ce3a
📒 Files selected for processing (3)
apps/web/src/components/Header.astroapps/web/src/css/global.cssapps/web/src/pages/ci_cd.astro
✅ Files skipped from review due to trivial changes (1)
- apps/web/src/components/Header.astro
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/pages/ci_cd.astro`:
- Around line 52-54: The hero uses an h3 ("What if builds happened...")
immediately after the page h1, skipping h2; update that element (the <h3> with
class "mb-4 text-2xl font-bold text-white") to an <h2> to restore proper heading
hierarchy, or replace it with a non-heading element (e.g., a styled <div> or
<p>) if it is purely presentational.
- Around line 79-82: The CTA anchor elements (e.g., the <a href="#how-it-works"
class="inline-flex items-center justify-center rounded-xl border border-gray-600
bg-gray-800 px-8 py-4 text-lg font-medium text-white ...">) only have hover
styles and lack visible keyboard focus; update their class list to include a
visible focus style (for example adding focus:outline-none focus-visible:ring-2
focus-visible:ring-offset-2 focus-visible:ring-indigo-500 or a similar
high-contrast ring/outline utility) so that keyboard users see a clear focus
indicator, and apply the same change to the other CTA anchors noted (lines
~350-353 and ~960-963). Ensure the focus style meets contrast and visibility
guidelines.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 61994fbe-05af-4f52-9a67-e6224865a4c7
📒 Files selected for processing (2)
apps/web/src/css/global.cssapps/web/src/pages/ci_cd.astro
✅ Files skipped from review due to trivial changes (1)
- apps/web/src/css/global.css
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Validation
Summary by CodeRabbit