fix: responsive form layouts, developer toolkit, and thread card overflow on mobile #2626
Conversation
WalkthroughThis pull request applies responsive layout and styling updates across four Svelte components: src/routes/(marketing)/(components)/developers-toolkit.svelte (restructures Build and Deploy groups from fixed horizontal layout to breakpoint-aware stacking, changes button padding to responsive values, replaces inline labels with spans hidden on small screens and visible on lg, and makes group gap elements responsive), src/routes/contact-us/+page.svelte and src/routes/startups/+page.svelte (constrains and centers forms with mx-auto, w-full, max-w-xl and makes submit buttons full-width by default and auto on large screens), and src/routes/threads/ThreadCard.svelte (adds min-w-0, flex-wrap, truncate, overflow and hyphenation rules and shrink-0 to prevent layout overflow and improve text wrapping). No exported/public declarations or runtime logic were changed. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/routes/threads/ThreadCard.svelte (1)
14-22: Remove undefined utility classwrap-break-wordfrom line 16.The class
wrap-break-wordis not defined anywhere in the codebase and is not a standard Tailwind utility. The word-breaking styling is already applied to theh3element via the component's<style>block (lines 71–76), making this class redundant and non-functional. Removewrap-break-wordfrom the class attribute.
🧹 Nitpick comments (1)
src/routes/threads/ThreadCard.svelte (1)
33-37: Add key to each block for proper list handling.The ESLint warning correctly identifies that this
{#each}block is missing a key. While this might work for stable data, it's best practice to add a key for proper DOM reconciliation.Apply this diff to add a key:
-{#each thread.tags ?? [] as tag} +{#each thread.tags ?? [] as tag, index (index)} <li class="min-w-0"> <div class="web-tag truncate">{tag}</div> </li> {/each}Note: If tags are guaranteed to be unique, you could use
(tag)as the key instead of(index).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/routes/(marketing)/(components)/developers-toolkit.svelte(4 hunks)src/routes/contact-us/+page.svelte(3 hunks)src/routes/startups/+page.svelte(4 hunks)src/routes/threads/ThreadCard.svelte(4 hunks)
🧰 Additional context used
🪛 ESLint
src/routes/threads/ThreadCard.svelte
[error] 33-37: Each block should have a key
(svelte/require-each-key)
🔇 Additional comments (12)
src/routes/contact-us/+page.svelte (3)
145-145: LGTM: Proper form width constraint and centering.The
mx-auto max-w-xlcombination properly centers and constrains the form width, aligning with the PR's objective of consistent layout across breakpoints.
194-217: LGTM: Consistent button wrapper constraint.The button wrapper now shares the same
max-w-xlconstraint as the form above, creating a unified, centered layout. Thejustify-betweenproperly spaces the error message and submit button.
210-216: LGTM: Correct Tailwind v4 important syntax and responsive button behavior.The button properly uses
w-full!andlg:w-auto!(with!at the end per Tailwind v4 requirements), making it full-width on mobile and compact on desktop as intended.src/routes/(marketing)/(components)/developers-toolkit.svelte (2)
63-79: LGTM: Responsive padding prevents overflow on small screens.The progressive padding scale (
px-0.5 sm:px-1 md:px-2 lg:px-3) provides minimal spacing on mobile to prevent horizontal overflow while scaling up appropriately on larger viewports. This directly addresses the stated objective of preventing cramped spacing and overflow in the Developer Toolkit.
70-77: LGTM: Hidden labels prevent overflow while preserving accessibility.Wrapping labels with
hidden lg:inlineeffectively prevents text overflow on small screens while keeping icons visible and tappable. Thealtattribute on the images maintains accessibility for screen readers.Also applies to: 100-107
src/routes/threads/ThreadCard.svelte (3)
31-38: LGTM: Proper flex wrapping and tag truncation.The combination of
min-w-0,flex-wrap, andtruncateeffectively prevents tag overflow on small screens. Tags will wrap to new lines when needed, and individual long tags will truncate with ellipsis.
40-46: LGTM: Reply counter protected from shrinking.The
shrink-0class prevents the reply counter from being compressed in flex layouts, ensuring it remains readable at its intended size.
51-92: LGTM: Comprehensive overflow handling and word-breaking rules.The CSS properly implements:
overflow: hiddenon the card to prevent horizontal scroll- Multiple word-breaking properties (
overflow-wrap,word-wrap,word-break) for cross-browser compatibilityhyphens: autofor improved readability- Tag truncation styles to support the
truncateclassThese changes effectively address the stated objective of keeping long URLs and text within card boundaries on mobile.
src/routes/startups/+page.svelte (4)
319-319: LGTM: Formatting cleanup.Removed unnecessary leading space in the class attribute.
402-406: LGTM: Consistent form width constraint.The form wrapper uses the same
mx-auto max-w-xlpattern as the contact-us page, providing consistent responsive behavior across both forms.
460-478: LGTM: Consistent button wrapper with clever line break control.The button wrapper matches the contact-us pattern. The
hidden sm:blockon the<br>(line 466) is a clever solution to control text wrapping—the reCAPTCHA disclaimer stays on one line on mobile and breaks appropriately on larger screens.
485-491: LGTM: Correct Tailwind v4 syntax and consistent responsive behavior.The submit button uses proper Tailwind v4 important syntax (
w-full!andlg:w-auto!) and matches the responsive behavior implemented in the contact-us form.
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)
src/routes/threads/ThreadCard.svelte (1)
31-47: Verify the responsive layout changes and add a key to the each block.The
min-w-0,flex-wrap,truncate, andshrink-0utilities properly handle tag wrapping and prevent layout overflow on mobile. However, the{#each}block at lines 33-37 is missing akeydirective, which can cause DOM reconciliation issues when the tag list changes.Apply this diff to add a unique key:
- {#each thread.tags ?? [] as tag} + {#each thread.tags ?? [] as tag (tag)} <li class="min-w-0"> <div class="web-tag truncate">{tag}</div> </li>Note: This assumes tags are unique. If tags can be duplicated, use
(tag, index)as the key instead, though this is less ideal for performance.
♻️ Duplicate comments (1)
src/routes/threads/ThreadCard.svelte (1)
24-29: Remove the undefinedu-break-wordutility class.The
u-break-wordclass on line 25 was previously flagged as non-existent in the codebase and marked as "Addressed in commit 4630e29", but it still appears in the code. This class has no effect. The word-breaking behavior is already handled by the CSS rules in.thread p {}at lines 64-69.Apply this diff to remove the undefined class:
- class="web-main-body-500 u-margin-block-start-4 u-break-word min-w-0" + class="web-main-body-500 u-margin-block-start-4 min-w-0"
🧹 Nitpick comments (2)
src/routes/threads/ThreadCard.svelte (2)
64-76: Consider removing redundant inherited word-breaking rules.Lines 64-76 re-declare
overflow-wrap,word-wrap,word-break, andhyphenson childpandh3elements, but these properties are already defined on the parent.thread(lines 60-62) and are inherited. Unless there's a specific override needed, these declarations are redundant.Additionally,
word-break: break-word(lines 67, 74) is non-standard, as noted in the previous comment.Apply this diff to remove the redundant rules:
overflow-wrap: break-word; word-wrap: break-word; - word-break: break-word; - p { - overflow-wrap: break-word; - word-wrap: break-word; - word-break: break-word; - hyphens: auto; - } - - h3 { - overflow-wrap: break-word; - word-wrap: break-word; - word-break: break-word; - hyphens: auto; - } + p, + h3 { + hyphens: auto; + } }Note: Keep the
hyphens: autoon children only if you want to be explicit, or remove it entirely if inheritance from.threadis sufficient.
87-92: Remove duplication between CSS and Tailwind utility.The
.web-tagCSS at lines 87-92 duplicates thetruncateutility class applied on line 35. Thetruncateutility already providesoverflow: hidden,text-overflow: ellipsis, andwhite-space: nowrap.Option 1: Remove the
truncateclass from line 35 and keep only the CSS:- <div class="web-tag truncate">{tag}</div> + <div class="web-tag">{tag}</div>Option 2: Keep the
truncateutility and simplify the CSS to only addmax-width:.web-tag { max-width: 100%; - overflow: hidden; - text-overflow: ellipsis; - white-space: nowrap; }Option 2 is preferred as it leverages Tailwind utilities while adding only the additional constraint needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/routes/threads/ThreadCard.svelte(4 hunks)
🧰 Additional context used
🪛 ESLint
src/routes/threads/ThreadCard.svelte
[error] 33-37: Each block should have a key
(svelte/require-each-key)
🔇 Additional comments (2)
src/routes/threads/ThreadCard.svelte (2)
14-22: LGTM! Proper flex overflow handling.The
min-w-0utilities correctly enable the flex container and title to shrink below their min-content width, preventing overflow on narrow viewports.
51-55: LGTM! Proper overflow containment.The
overflow: hiddenon.web-cardprevents content from visually escaping the card boundaries, working in conjunction with the word-breaking and wrapping rules.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/routes/threads/ThreadCard.svelte (2)
35-36: Avoid duplicating truncation logic between Tailwind and.web-tagCSS.
.web-tagnow appliesoverflow: hidden,text-overflow: ellipsis, andwhite-space: nowrap, which overlaps with thetruncateutility on the same element. To keep a single source of truth, consider droppingtruncatefrom the markup and relying on the.web-tagrule:- <div class="web-tag truncate">{tag}</div> + <div class="web-tag">{tag}</div>This keeps behavior while simplifying the class list.
Also applies to: 84-89
52-55: Clipping card overflow is reasonable for this layout.Adding
overflow: hiddento.web-cardwill prevent child content from bleeding outside the card on small screens, which matches the goal of eliminating horizontal scroll. Just be aware this will also clip any internal overflow (e.g., extra-long focus rings or box-shadows) if those are added later.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/routes/threads/ThreadCard.svelte(4 hunks)
🧰 Additional context used
🪛 ESLint
src/routes/threads/ThreadCard.svelte
[error] 33-37: Each block should have a key
(svelte/require-each-key)
🔇 Additional comments (6)
src/routes/threads/ThreadCard.svelte (6)
14-16: Header flex adjustments look correct.Making the header row flex with
min-w-0and giving the title a flexible width should prevent it from forcing horizontal overflow while still letting it grow naturally. No changes needed here.
24-29: Body text overflow handling is appropriate.Adding
min-w-0to the content paragraph complements the new.threadword-wrapping rules and helps avoid horizontal scrolling on small screens. Looks good.
31-36: Tag container flex/wrap behavior is well tuned.Using
min-w-0plusflex-wrapon the tags container and items should keep tags within the card while allowing natural wrapping across lines. This aligns well with the overflow constraints.
40-46: Reply button flex behavior is appropriate.Keeping the replies button as a
flexcontainer withshrink-0is a good way to ensure the counter and icon remain visible and aligned while the textual content and tags wrap independently.
57-73: Thread text wrapping and hyphenation rules are solid.
max-width: 100%plusoverflow-wrap/word-wrapandhyphens: autoon bothpandh3should handle long URLs and titles cleanly within the card, without relying on nonstandardword-breakvalues. This is a nice, standards-compliant improvement.
84-89: Global.web-tagtruncation matches the PR’s goals.Constraining
.web-tagwithmax-width: 100%, hidden overflow, ellipsis, and no wrapping aligns with the intent to keep tags on one line and avoid layout shifts in tight spaces. No issues here.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/routes/threads/ThreadCard.svelte (1)
25-25: Reconfirmu-break-wordutility or drop it as redundant
u-break-wordis still present on the paragraph, but long‑content wrapping is already handled via the.threadSCSS rules below. Ifu-break-wordremains an undefined utility, it adds noise without effect; if it was added later, it’s now somewhat redundant.Either:
- Confirm it’s defined and intentionally kept, or
- Remove it to rely solely on the
.threadstyles.You can re‑check its definition with:
#!/bin/bash # Verify any definitions/usages of u-break-word rg -n "u-break-word" src/app.css src/scss || echo "No matches found for u-break-word"
🧹 Nitpick comments (1)
src/routes/threads/ThreadCard.svelte (1)
31-36: Tag flex/wrap + truncation behavior matches mobile overflow goalsThe combination of
flex min-w-0 flex-wrapon the container,min-w-0on<li>, and.web-tag truncate(plus the SCSS below) should keep tags within the card, let chips wrap to new lines, and ellipsize overly long tag text. This aligns well with the “no horizontal scroll, clean wrapping” requirement.If tags are guaranteed unique per thread, you could simplify the key to
(tag), but the current(tag + index)is functionally fine.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/routes/threads/ThreadCard.svelte(4 hunks)
🔇 Additional comments (3)
src/routes/threads/ThreadCard.svelte (3)
14-17: min-w-0 on flex row and heading correctly prevents title overflowUsing
min-w-0on the flex container andh3withflex-1is the right pattern to allow the title to shrink/wrap instead of forcing horizontal scroll in tight layouts. No issues here.
41-46: Reply counter shrink-0 avoids clipping in tight flex layoutsAdding
flex shrink-0 items-centeron the replies button is a good call: it prevents the icon/counter cluster from being squashed when the title/description grow, which directly helps maintain readability on small screens.
54-61: Text wrapping and truncation rules look consistent and robust
overflow: hiddenon.web-cardplus the.threadword‑wrapping and hyphenation (and the.web-tagtruncation styles) give you strong control over long titles, content, URLs, and tags so they stay within card bounds without horizontal scrolling. This matches the thread‑card objectives and should behave well across breakpoints.Also applies to: 63-73, 84-89
HarshMN2345
left a comment
HarshMN2345
left a comment
There was a problem hiding this comment.
Thanks @AbhiVarde for the contribution 🙌
Really appreciate you taking the time to work on this, great job so far!
A few frontend improvement suggestions for the Startups page from my side:
Increase spacing between the two containers (around 8px would look better)
Make the BUILD container full width
Center align both containers to the page for better layout balance
Remove the line on the right side of the BUILD container
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/routes/(marketing)/(components)/developers-toolkit.svelte (1)
67-79: Label hiding and padding achieve the mobile overflow goalsUsing responsive padding (
px-0.5 sm:px-1 md:px-2 lg:px-3) plusw-fiton the pill links, and moving the textual label into<span class="hidden lg:inline">, keeps the icons tappable while preventing text overflow on small screens. Theimgalttext still provides an accessible name for the links when labels are hidden, so this change is consistent with the linked issue’s requirements.If this pattern is reused elsewhere, consider extracting a small
ProductLinkcomponent to avoid duplication between the Build and Deploy groups.Also applies to: 97-109
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/routes/(marketing)/(components)/developers-toolkit.svelte(4 hunks)
🧰 Additional context used
🪛 ESLint
src/routes/(marketing)/(components)/developers-toolkit.svelte
[error] 66-66: Unexpected href link without resolve().
(svelte/no-navigation-without-resolve)
🔇 Additional comments (2)
src/routes/(marketing)/(components)/developers-toolkit.svelte (2)
55-60: Responsive stacking and divider behavior look correctThe outer wrapper now stacks the Build/Deploy groups vertically on small screens and switches to a horizontal, wrapping layout on
lgwith the dashed divider only visible on large screens. Thew-full→lg:w-autobehavior on the Build pill matches the objective of keeping things centered and compact on desktop while avoiding overflow on mobile.Also applies to: 86-86
64-67: I'm unable to verify this review comment because the repository is currently inaccessible—the cloning operation failed. Without access to the codebase, I cannot:
- Confirm whether the
svelte/no-navigation-without-resolveESLint rule is configured in the project- Verify if
$app/pathsand theresolve()function are available in this SvelteKit project- Validate that the code at lines 64-67 and 94-97 matches the provided snippet
- Check how
resolve()is used elsewhere in the codebase- Determine whether the suggested fix is appropriate for this project
Manual verification required: Please confirm the following:
- Is
svelte/no-navigation-without-resolveconfigured in your ESLint setup?- Does your project use SvelteKit's
$app/pathsmodule?- Are internal navigation links in other parts of the codebase wrapped with
resolve()?
|
Hey @HarshMN2345, thanks! Changes applied, ready for another look 🙏 |
2 participants
What does this PR do?
Problem
Multiple pages had responsive layout and overflow issues on mobile and small screens (< 1024px):
Solution
Implemented comprehensive responsive fixes across all affected components:
max-w-xlconstraint to form and button wrappersw-full! lg:w-auto!)< lg) while keeping icons visible and tappablepx-0.5topx-3based on screen sizemin-w-0,flex-wrap, andbreak-wordutilitiesoverflow: hiddento card container to prevent layout shiftsChanges Made
Files Modified:
src/routes/contact-us/+page.sveltemx-auto max-w-xlcontainermax-w-xlconstraintw-full! lg:w-auto!classes to submit button for responsive widthsrc/routes/startups/+page.sveltehidden sm:blockfor better mobile layoutsrc/routes/(marketing)/(components)/developers-toolkit.sveltehidden lg:inlinepx-0.5 sm:px-1 md:px-2 lg:px-3src/routes/threads/ThreadCard.sveltemin-w-0to parent containers to enable proper text wrappingflex-wrapfor tags and contentoverflow: hiddento card containertruncateto tags for long tag namesshrink-0to prevent compressionTest Plan
Screenshots
Related PRs and Issues
Fixes #2599
Have you read the Contributing Guidelines?
Yes, I have read and followed the contributing guidelines.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.