Fix: iPad navbar spacing and CTA and customers page#2750
Conversation
WalkthroughChanged responsive and layout behavior across header and category UI. In src/scss/7-components/_main-header.scss a break2 media block was replaced with new spacing, typography, visibility, and sizing rules (padding, gaps, font sizes, inline tag hidden, button/CTA adjustments, logo margin). In src/scss/7-components/_mobile-header.scss the responsive token was changed from break3open to break2open. src/markdoc/layouts/Category.svelte simplifies and restructures the category header and surrounding container markup. src/scss/8-sequences/_category-header.scss adds/adjusts category header and page classes, modifies flex layouts across breakpoints. Total lines changed: +70 / -25. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
Verify each finding against the current code and only fix it if needed.
In `@src/scss/7-components/_main-header.scss`:
- Around line 138-141: The current rules only toggle .cta-full and .cta-short
inside the iPad media query, so at larger viewports both can be visible; add
base-level display rules for .web-main-header-cta .cta-full and
.web-main-header-cta .cta-short (e.g., set .cta-short { display: none; } and
optionally .cta-full { display: inline; }) outside any media query, then keep
the existing media-query override that swaps them for the iPad breakpoint to
ensure predictable default and breakpoint behavior.
- Around line 132-136: The selector .#{$p}-button inside the main header will
target all buttons at this breakpoint; decide whether that broad scope is
intentional — if you only want to resize the CTA, narrow the rule to a more
specific selector such as .web-main-header-cta .#{$p}-button (or another
CTA-specific class) instead of the current .#{$p}-button so other header buttons
(sign-in, secondary actions) aren’t affected; update the selector in the
.#{$p}-button rule accordingly or confirm leaving it global.
🧹 Nitpick comments (1)
🤖 Fix all nitpicks with AI agents
Verify each finding against the current code and only fix it if needed. In `@src/scss/7-components/_main-header.scss`: - Around line 132-136: The selector .#{$p}-button inside the main header will target all buttons at this breakpoint; decide whether that broad scope is intentional — if you only want to resize the CTA, narrow the rule to a more specific selector such as .web-main-header-cta .#{$p}-button (or another CTA-specific class) instead of the current .#{$p}-button so other header buttons (sign-in, secondary actions) aren’t affected; update the selector in the .#{$p}-button rule accordingly or confirm leaving it global.src/scss/7-components/_main-header.scss (1)
132-136: Broad button override — confirm scope is intentional.
.#{$p}-buttoninside the header selector will resize all buttons in the main header at this breakpoint (e.g., sign-in, other secondary actions), not just the CTA. If that's the intent, this is fine; otherwise, scope it to a more specific selector like.web-main-header-cta .#{$p}-button.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/scss/7-components/_main-header.scss` around lines 132 - 136, The selector .#{$p}-button inside the main header will target all buttons at this breakpoint; decide whether that broad scope is intentional — if you only want to resize the CTA, narrow the rule to a more specific selector such as .web-main-header-cta .#{$p}-button (or another CTA-specific class) instead of the current .#{$p}-button so other header buttons (sign-in, secondary actions) aren’t affected; update the selector in the .#{$p}-button rule accordingly or confirm leaving it global.
| .web-main-header-cta { | ||
| .cta-full { display: none; } | ||
| .cta-short { display: inline; } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n 'cta-short|cta-full' --type=scss --type=css -C 3Repository: appwrite/website
Length of output: 88
🏁 Script executed:
rg -n 'cta-short|cta-full' -g '*.scss' -C 3Repository: appwrite/website
Length of output: 559
🏁 Script executed:
cat -n src/scss/7-components/_main-header.scss | head -150Repository: appwrite/website
Length of output: 4732
🏁 Script executed:
rg -n '\.web-main-header-cta|\.cta-full|\.cta-short' -g '*.scss' -C 5Repository: appwrite/website
Length of output: 965
🏁 Script executed:
rg -n 'cta-full|cta-short' -C 3 --max-count=30Repository: appwrite/website
Length of output: 559
Define base state for .cta-full and .cta-short outside the media query.
The toggle at lines 139-140 hides .cta-full and shows .cta-short only at the iPad breakpoint. For this pattern to work correctly, you must define display: none on .cta-short (and optionally display: inline on .cta-full) at the base level outside any media query. Without this, both elements will be visible at larger viewports.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/scss/7-components/_main-header.scss` around lines 138 - 141, The current
rules only toggle .cta-full and .cta-short inside the iPad media query, so at
larger viewports both can be visible; add base-level display rules for
.web-main-header-cta .cta-full and .web-main-header-cta .cta-short (e.g., set
.cta-short { display: none; } and optionally .cta-full { display: inline; })
outside any media query, then keep the existing media-query override that swaps
them for the iPad breakpoint to ensure predictable default and breakpoint
behavior.
There was a problem hiding this comment.
🤖 Fix all issues with AI agents
Verify each finding against the current code and only fix it if needed.
In `@src/scss/8-sequences/_category-header.scss`:
- Around line 12-20: Merge the two duplicate `@media` #{$break4open} blocks into a
single media query: combine the rules for &-content, &-title, &-description and
.#{$p}-input-text-search-wrapper so all styles for that breakpoint live
together; ensure you keep both sets of declarations (flex-direction,
align-items, justify-content, flex rules, white-space, the pxToRem
widths/paddings, and the .#{$p}-input-text-search-wrapper align-self and
margin-block-start) and preserve their intended specificity and order when
consolidating.
🧹 Nitpick comments (1)
🤖 Fix all nitpicks with AI agents
Verify each finding against the current code and only fix it if needed. In `@src/scss/8-sequences/_category-header.scss`: - Around line 12-20: Merge the two duplicate `@media` #{$break4open} blocks into a single media query: combine the rules for &-content, &-title, &-description and .#{$p}-input-text-search-wrapper so all styles for that breakpoint live together; ensure you keep both sets of declarations (flex-direction, align-items, justify-content, flex rules, white-space, the pxToRem widths/paddings, and the .#{$p}-input-text-search-wrapper align-self and margin-block-start) and preserve their intended specificity and order when consolidating.src/scss/8-sequences/_category-header.scss (1)
12-20: Duplicate@media #{$break4open}blocks should be merged.Lines 12–16 and 17–20 both target the same
$break4openbreakpoint. Consolidate them into a single block to reduce redundancy and avoid confusion about rule ordering.♻️ Proposed merge
- `@media` #{$break4open} { - &-content { flex-direction:row; align-items:flex-start; } - &-title { flex:1 1 auto; white-space:nowrap; } - &-description { flex:0 0 pxToRem(520); padding-block-start:pxToRem(10); } - } - `@media` #{$break4open} { - &-content { justify-content:space-between; } - .#{$p}-input-text-search-wrapper { align-self:end; margin-block-start:pxToRem(40); } - } + `@media` #{$break4open} { + &-content { flex-direction:row; align-items:flex-start; justify-content:space-between; } + &-title { flex:1 1 auto; white-space:nowrap; } + &-description { flex:0 0 pxToRem(520); padding-block-start:pxToRem(10); } + .#{$p}-input-text-search-wrapper { align-self:end; margin-block-start:pxToRem(40); } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/scss/8-sequences/_category-header.scss` around lines 12 - 20, Merge the two duplicate `@media` #{$break4open} blocks into a single media query: combine the rules for &-content, &-title, &-description and .#{$p}-input-text-search-wrapper so all styles for that breakpoint live together; ensure you keep both sets of declarations (flex-direction, align-items, justify-content, flex rules, white-space, the pxToRem widths/paddings, and the .#{$p}-input-text-search-wrapper align-self and margin-block-start) and preserve their intended specificity and order when consolidating.
2 participants
What does this PR do?
before:
after:
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)
Related PRs and Issues
(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)
Have you read the Contributing Guidelines on issues?
(Write your answer here.)
Summary by CodeRabbit
Style
Refactor