-
Notifications
You must be signed in to change notification settings - Fork 10.8k
chore: add autocomplete for inputs - name, phone, location #24422
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
chore: add autocomplete for inputs - name, phone, location #24422
Conversation
@dhairyashiil is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis change adds HTML autocomplete attributes to several input components. PhoneInput (BasePhoneInput and BasePhoneInputWeb) inputs now include autoComplete="tel". Form-builder InputField components set autoComplete="name" for fullName, "given-name" for firstName, and "family-name" for lastName; email and url variants set autoComplete="email" and "url" respectively. AddressInput sets autoComplete="address-line1". No other logic or handlers were modified. Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/features/form-builder/Components.tsx (1)
178-178
: Consider explicit field name matching for robustness.The current ternary assumes only
firstName
andlastName
fields exist. If the variant includes additional fields (e.g.,middleName
,suffix
), they would incorrectly receive"family-name"
. While this may work if variants are strictly controlled, an explicit check would be more maintainable.Apply this diff for more explicit field name handling:
- autoComplete={variantField.name === "firstName" ? "given-name" : "family-name"} + autoComplete={ + variantField.name === "firstName" + ? "given-name" + : variantField.name === "lastName" + ? "family-name" + : undefined + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/features/components/phone-input/PhoneInput.tsx
(2 hunks)packages/features/form-builder/Components.tsx
(2 hunks)packages/ui/components/address/AddressInput.tsx
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()
for text localization in frontend code; direct text embedding should trigger a warning
Files:
packages/features/components/phone-input/PhoneInput.tsx
packages/features/form-builder/Components.tsx
packages/ui/components/address/AddressInput.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()
in hot paths like loops
Files:
packages/features/components/phone-input/PhoneInput.tsx
packages/features/form-builder/Components.tsx
packages/ui/components/address/AddressInput.tsx
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/components/phone-input/PhoneInput.tsx
packages/features/form-builder/Components.tsx
packages/ui/components/address/AddressInput.tsx
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (3)
packages/ui/components/address/AddressInput.tsx (1)
25-25
: LGTM!The
autoComplete="address-line1"
attribute correctly enables browser autofill for the street address field, aligning with HTML autocomplete standards.packages/features/components/phone-input/PhoneInput.tsx (1)
71-71
: LGTM!Adding
autoComplete: "tel"
to bothBasePhoneInput
andBasePhoneInputWeb
correctly enables browser autofill for telephone numbers, consistent with HTML standards.Also applies to: 122-122
packages/features/form-builder/Components.tsx (1)
148-148
: LGTM!The
autoComplete="name"
attribute correctly enables browser autofill for the full name field, aligning with HTML autocomplete standards.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please address coderabbit suggestions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/features/form-builder/Components.tsx (2)
217-224
: Consider adding autocomplete for email field.While not in the original PR scope, adding
autoComplete="email"
to the email InputField would improve consistency and user experience, similar to the name field improvements in this PR.Apply this diff:
return ( <InputField type="email" id={props.name} noLabel={true} + autoComplete="email" {...props} onChange={(e) => props.setValue(e.target.value)} /> );
554-554
: Consider adding autocomplete for URL field.For completeness, you could add
autoComplete="url"
to the URL field as well, though this is less commonly used than name or email autocomplete.Apply this diff:
- factory: (props) => { - return <Widgets.TextWidget type="url" noLabel={true} {...props} />; + factory: (props) => { + return <Widgets.TextWidget type="url" noLabel={true} autoComplete="url" {...props} />; },
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/features/form-builder/Components.tsx
(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()
for text localization in frontend code; direct text embedding should trigger a warning
Files:
packages/features/form-builder/Components.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()
in hot paths like loops
Files:
packages/features/form-builder/Components.tsx
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/features/form-builder/Components.tsx
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Tests / Unit
- GitHub Check: Linters / lint
- GitHub Check: Type check / check-types
🔇 Additional comments (2)
packages/features/form-builder/Components.tsx (2)
148-148
: LGTM! Correct autocomplete for full name.The
autoComplete="name"
attribute is the correct HTML5 autocomplete value for a single full name input field.
178-184
: LGTM! Correct conditional autocomplete for name variants.The conditional logic correctly maps:
firstName
→"given-name"
lastName
→"family-name"
- Other fields →
undefined
These are the correct HTML5 autocomplete values for split name fields.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked our email component on the booking page and it also doesn't have autocomplete
property set.
<input id="email" placeholder="" class="rounded-[10px] border font-normal bg-default border-default text-default placeholder:text-muted hover:border-emphasis focus:shadow-outline-gray-focused shadow-outline-gray-rested transition h-8 px-3 py-2 text-sm w-full focus:border-subtle focus:ring-brand-default focus:ring-2 disabled:bg-subtle disabled:hover:border-subtle disabled:cursor-not-allowed" type="email" value="" name="email">
Thank you for the review. I’ll set the autocomplete property in the email component and check if it’s missing elsewhere, especially on other booking page fields. |
/> | ||
<Input | ||
{...rest} | ||
autoComplete="address-line1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we could also use streetAddress but this is also fine
Add autocomplete for input fields like name, phone, location
user posted this on x: https://x.com/swhillance/status/1977129174075949556