27 miscellaneous fixes (family form + storefront)#75
Merged
Conversation
Contributor
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughAdds max-character support to form inputs/textarea, enforces child age as an integer 1–18 via a numeric input, converts several blurb/notes validations to 150-character limits, and wires schema and hook updates to use the new helpers. ChangesForm validation & UI
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
app/src/components/storefront/StorefrontNavbar.tsx (1)
104-112: Optional: improve screen reader semantics for the cart badge.The badge text is read inline (e.g., "Your Cart 3") which is OK but could be more explicit. Consider marking the visual badge as
aria-hiddenand conveying the count viaaria-labelon the button, so assistive tech reads something like "Your Cart, 3 items".♻️ Proposed refinement
- <Button variant="default" className="relative"> + <Button + variant="default" + className="relative" + aria-label={ + cartCount > 0 + ? `Your Cart, ${cartCount} item${cartCount === 1 ? "" : "s"}` + : "Your Cart" + } + > Your Cart <ShoppingCartIcon className="ml-2" /> {cartCount > 0 && ( - <span className="absolute -top-2 -right-2 bg-red-600 text-white text-xs font-bold rounded-full h-5 min-w-[20px] px-1 flex items-center justify-center"> + <span + aria-hidden="true" + className="absolute -top-2 -right-2 bg-red-600 text-white text-xs font-bold rounded-full h-5 min-w-[20px] px-1 flex items-center justify-center" + > {cartCount} </span> )} </Button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/storefront/StorefrontNavbar.tsx` around lines 104 - 112, The visual cart badge currently reads inline; update the Button component (the "Your Cart" button that renders ShoppingCartIcon and the span showing {cartCount}) to mark the badge span as aria-hidden and move the accessible count into the Button's accessible name by setting/constructing an aria-label on Button (e.g., "Your Cart, {cartCount} item(s)") that handles pluralization when cartCount === 1 vs >1; keep the visual span for sighted users but ensure it has aria-hidden="true" so screen readers rely on the aria-label.app/src/components/form/sections/GiftDetails.tsx (2)
194-205: Optional: extract the 150-word validator to a shared helper.The same word-count logic now lives in three places across
GiftDetails.tsx(gift + backup gift) andChildInfo.tsx(child blurb). A small shared helper (e.g.maxWordsValidator(150, "blurb")) would keep the threshold/error message consistent and reduce drift.// e.g. app/src/lib/formValidators.ts export const maxWordsValidator = (max: number, message = `Please keep your blurb to ${max} words or less`) => ({ value }: { value: string | undefined }) => { if (!value) return undefined; const wordCount = value.trim().split(/\s+/).filter(Boolean).length; return wordCount > max ? message : undefined; };Also applies to: 314-325
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/form/sections/GiftDetails.tsx` around lines 194 - 205, Extract the duplicated 150-word validator into a shared helper function (e.g., maxWordsValidator) and replace the inline validators in GiftDetails (both gift and backup gift validators) and ChildInfo (child blurb validator) to call that helper; implement maxWordsValidator(max:number, message?:string) that returns the validator signature used by the components (accepting { value }) and performs the trim/split/filter length check, then import and use maxWordsValidator(150) in the validators props of the affected fields (replace the inline onChange validator logic with a reference to the helper).
194-205: Word-count validator ignoresdisabled, unlike sibling validators in this file.The other validators in this file (
giftUrl,giftName) gate themselves withdisabled ? undefined : { onChange: ... }, but the two newfamilyPublicNotesvalidators always run. In practice it's unlikely to fire while disabled (no value changes), but it's worth aligning for consistency.♻️ Proposed alignment
- validators={{ - onChange: ({ value }) => { - if (!value) return undefined; - const wordCount = value - .trim() - .split(/\s+/) - .filter(Boolean).length; - if (wordCount > 150) - return "Please keep your blurb to 150 words or less"; - return undefined; - }, - }} + validators={ + disabled + ? undefined + : { + onChange: ({ value }) => { + if (!value) return undefined; + const wordCount = value + .trim() + .split(/\s+/) + .filter(Boolean).length; + if (wordCount > 150) + return "Please keep your blurb to 150 words or less"; + return undefined; + }, + } + }Also applies to: 314-325
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/form/sections/GiftDetails.tsx` around lines 194 - 205, The word-count validators for familyPublicNotes currently always run; change them to mirror the pattern used by giftUrl/giftName by gating the validators with the disabled flag (e.g., set validators to disabled ? undefined : { onChange: (...) => { ... } }) so they return undefined when disabled; update both occurrences of the familyPublicNotes validators (the validator blocks around the word-count logic) to use this disabled ? undefined : pattern and keep the existing onChange implementation unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/components/form/sections/ChildInfo.tsx`:
- Around line 162-176: The age field validation allows decimals and malformed
values; update the input and validators in ChildInfo to enforce integers and
robust numeric checks: add step="1" to the <input> and in the validators
(validators.onChange and validators.onBlur) parse the value with const n =
Number(value); return "Age is required" for empty/undefined, return "Age must be
an integer" if !Number.isFinite(n) or !Number.isInteger(n), return "Age must be
between 1 and 18" if n < 1 || n > 18, otherwise return undefined; apply the same
tightened validator logic to the other age block referenced (the similar
validators around the later section).
In `@app/src/components/storefront/StorefrontNavbar.tsx`:
- Around line 23-25: Pass the cart data/count from StorefrontNavbar into the
mobile sidebar and render the same badge there: compute cartCount using
useLocalCartData() in StorefrontNavbar (as already done) and add a prop like
cartCount or localCart to the StorefrontMobileSidebar invocation; then update
the StorefrontMobileSidebar component to accept that prop and render the badge
next to the "Your Cart" menu item using the same badge markup/styling used in
the desktop block (refer to the desktop badge rendering in StorefrontNavbar
lines that render the cart badge) so mobile shows the item count.
---
Nitpick comments:
In `@app/src/components/form/sections/GiftDetails.tsx`:
- Around line 194-205: Extract the duplicated 150-word validator into a shared
helper function (e.g., maxWordsValidator) and replace the inline validators in
GiftDetails (both gift and backup gift validators) and ChildInfo (child blurb
validator) to call that helper; implement maxWordsValidator(max:number,
message?:string) that returns the validator signature used by the components
(accepting { value }) and performs the trim/split/filter length check, then
import and use maxWordsValidator(150) in the validators props of the affected
fields (replace the inline onChange validator logic with a reference to the
helper).
- Around line 194-205: The word-count validators for familyPublicNotes currently
always run; change them to mirror the pattern used by giftUrl/giftName by gating
the validators with the disabled flag (e.g., set validators to disabled ?
undefined : { onChange: (...) => { ... } }) so they return undefined when
disabled; update both occurrences of the familyPublicNotes validators (the
validator blocks around the word-count logic) to use this disabled ? undefined :
pattern and keep the existing onChange implementation unchanged.
In `@app/src/components/storefront/StorefrontNavbar.tsx`:
- Around line 104-112: The visual cart badge currently reads inline; update the
Button component (the "Your Cart" button that renders ShoppingCartIcon and the
span showing {cartCount}) to mark the badge span as aria-hidden and move the
accessible count into the Button's accessible name by setting/constructing an
aria-label on Button (e.g., "Your Cart, {cartCount} item(s)") that handles
pluralization when cartCount === 1 vs >1; keep the visual span for sighted users
but ensure it has aria-hidden="true" so screen readers rely on the aria-label.
🪄 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: 970076a9-abbf-48c0-88c9-0b17df44d960
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
app/src/components/form/sections/ChildInfo.tsxapp/src/components/form/sections/GiftDetails.tsxapp/src/components/storefront/SiblingsCarousel.tsxapp/src/components/storefront/StorefrontNavbar.tsx
ArnavGupta23
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@app/src/components/form/FormComponents.tsx`:
- Around line 257-263: The immediate max-character error is computed even when
the field is disabled; update the conditional that defines
immediateMaxCharacterError to also require the field is not disabled (e.g., add
&& !disabled to the predicate alongside
showMaxCharactersErrorImmediately/maxCharacters/characterCount), and make the
same change to the other occurrence (the block around the second max-length
check referenced in the comment) so disabled/read-only fields do not show
immediate red errors.
🪄 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: 83799367-67d0-48e4-812a-642bf827f816
📒 Files selected for processing (5)
app/src/components/form/FormComponents.tsxapp/src/components/form/sections/ChildInfo.tsxapp/src/components/form/sections/GiftDetails.tsxapp/src/hooks/family-form/formHooks.tsapp/src/lib/formSchemas.ts
Comment on lines
+257
to
+263
| const immediateMaxCharacterError = | ||
| showMaxCharactersErrorImmediately && | ||
| maxCharacters !== undefined && | ||
| characterCount > maxCharacters | ||
| ? (maxCharactersErrorMessage ?? | ||
| `Please keep this field to ${maxCharacters} characters or less`) | ||
| : undefined; |
Contributor
There was a problem hiding this comment.
Suppress immediate max-length errors for disabled fields.
Immediate max-length errors are computed even when the field is disabled, so read-only forms can render red error states users cannot fix. This also conflicts with the disabled-state counter behavior at Line 398.
💡 Suggested fix
const immediateMaxCharacterError =
+ !disabled &&
showMaxCharactersErrorImmediately &&
maxCharacters !== undefined &&
characterCount > maxCharacters
? (maxCharactersErrorMessage ??
`Please keep this field to ${maxCharacters} characters or less`)
: undefined; const immediateMaxCharacterError =
+ !disabled &&
showMaxCharactersErrorImmediately &&
maxCharacters !== undefined &&
characterCount > maxCharacters
? (maxCharactersErrorMessage ??
`Please keep this field to ${maxCharacters} characters or less`)
: undefined;Also applies to: 371-377
🤖 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 `@app/src/components/form/FormComponents.tsx` around lines 257 - 263, The
immediate max-character error is computed even when the field is disabled;
update the conditional that defines immediateMaxCharacterError to also require
the field is not disabled (e.g., add && !disabled to the predicate alongside
showMaxCharactersErrorImmediately/maxCharacters/characterCount), and make the
same change to the other occurrence (the block around the second max-length
check referenced in the comment) so disabled/read-only fields do not show
immediate red errors.
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
2 participants
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.
Pull Request
Description
In the family form submission, the child's age is a text box instead of a drop-down. The personal blurb and gift public notes now have a word limit of 150. In the storefront pages, child sibling cards are centered if there are fewer than 3, and the number of items in the cart appears above the cart button in the header.
Type of Change
Related Issues (put task name here from notion)
27 - Miscellaneous Fixes
Screenshots (If it is a front end feature screenshot is required)
Additional Notes
Profile review page fixes were made on a separate branch.
Summary by CodeRabbit
New Features
Improvements