Skip to content

Conversation

KartikLabhshetwar
Copy link

@KartikLabhshetwar KartikLabhshetwar commented Sep 17, 2025

What does this PR do?

Visual Demo (For contributors especially)

A visual demonstration is strongly recommended, for both the original and new change (video / image - any one).

before:

Screen.Recording.2025-09-17.at.2.10.34.PM.mov

after:

Screen.Recording.2025-09-17.at.2.10.34.PM.mov

Video Demo (if applicable):

  • Show screen recordings of the issue or feature.
  • Demonstrate how to reproduce the issue, the behavior before and after the change.

Image Demo (if applicable):

  • Add side-by-side screenshots of the original and updated change.
  • Highlight any significant change(s).

before:

Screenshot 2025-09-17 at 1 34 09 PM Screenshot 2025-09-17 at 1 34 18 PM

after:
Screenshot 2025-09-17 at 1 37 33 PM
Screenshot 2025-09-17 at 1 38 08 PM

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

  • Are there environment variables that should be set?
  • What are the minimal test data to have?
  • What is expected (happy path) to have (input and output)?
  • Any other important info that could help to test that PR

Checklist

  • I haven't read the contributing guide
  • My code doesn't follow the style guidelines of this project
  • I haven't commented my code, particularly in hard-to-understand areas
  • I haven't checked if my changes generate no new warnings

Copy link

vercel bot commented Sep 17, 2025

@KartikLabhshetwar is attempting to deploy a commit to the cal Team on Vercel.

A member of the Team first needs to authorize it.

@CLAassistant
Copy link

CLAassistant commented Sep 17, 2025

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

coderabbitai bot commented Sep 17, 2025

Walkthrough

Updates are confined to packages/ui/components/form/inputs/TextField.tsx. The Addon component’s destructured props are renamed (size → _size with default "md"; position → _position with default "start"), with no changes to usage. In the InputField path, when the input type is "email", the Input element conditionally adds focus styles: focus:border-subtle focus:ring-brand-default focus:ring-2. No exports or public API signatures change beyond the internal parameter renaming in the Addon function.

✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Pre-merge checks

✅ Passed checks (5 passed)
Check name Status Explanation
Title Check ✅ Passed The title "fix(ui/auth): align email focus ring with password/username" is concise and directly describes the primary change in the diff — aligning the email input focus style with the password/username fields — and therefore accurately summarizes the main intent of the PR for team history.
Linked Issues Check ✅ Passed The code change in packages/ui/components/form/inputs/TextField.tsx implements the linked issues' objective by conditionally adding the thicker brand focus classes ("focus:border-subtle focus:ring-brand-default focus:ring-2") for inputs with type === "email", and the other modification is a non-functional renaming of Addon destructured props that does not change behavior or exports.
Out of Scope Changes Check ✅ Passed No substantive out-of-scope changes were found; the only unrelated edit is a cosmetic renaming of Addon props (size → _size, position → _position) which the summary indicates is non-functional and does not alter the public API or behavior.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
Description Check ✅ Passed The PR description clearly relates to the changeset: it states the intent to align the email input focus ring with other auth fields, references the related issue numbers, and includes before/after visual assets and basic testing guidance, so it is on-topic and describes the change.

@graphite-app graphite-app bot added the community Created by Linear-GitHub Sync label Sep 17, 2025
@github-actions github-actions bot added authentication area: authentication, auth, google sign in, password, SAML, password reset, can't log in ui area: UI, frontend, button, form, input 🐛 bug Something isn't working labels Sep 17, 2025
@dosubot dosubot bot added the 🧹 Improvements Improvements to existing features. Mostly UX/UI label Sep 17, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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/ui/components/form/inputs/TextField.tsx (1)

77-84: Drop unused Addon props (size/position) or wire them up.

You renamed to _size/_position but never use them. Prefer removing them from the destructure (and ideally from the type + call sites) to avoid dead props and future confusion.

Apply this minimal diff here:

-const Addon = ({
-  children,
-  className,
-  error,
-  onClickAddon,
-  size: _size = "md",
-  position: _position = "start",
-}: AddonProps) => (
+const Addon = ({ children, className, error, onClickAddon }: AddonProps) => (

Additionally (outside this hunk), consider:

  • Removing size and position from AddonProps
  • Dropping size/position props at uses (Line 162 and Lines 191–195)
📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between fd87954 and 68de254.

📒 Files selected for processing (1)
  • packages/ui/components/form/inputs/TextField.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/ui/components/form/inputs/TextField.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/ui/components/form/inputs/TextField.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/ui/components/form/inputs/TextField.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). (1)
  • GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (1)
packages/ui/components/form/inputs/TextField.tsx (1)

216-220: Email focus ring aligned with Password/Username — LGTM.

The conditional adds the thicker brand ring on focus for type="email" and matches the add-on path’s ring-2 ring-brand-default.

Please sanity-check:

  • Light/dark themes and disabled/readOnly states on login/signup.
  • No visual clash with focus:shadow-outline-gray-focused (ring should take precedence).

@KartikLabhshetwar
Copy link
Author

KartikLabhshetwar commented Sep 17, 2025

hi @anikdhabal, @supalarry please take a look at this pr.

Copy link
Contributor

@kart1ka kart1ka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@KartikLabhshetwar
Copy link
Author

hi @supalarry, @anikdhabal is there any issue here?

@KartikLabhshetwar KartikLabhshetwar changed the title fix(ui/auth): align email focus ring with password/username fix: align email focus ring with password/username Sep 19, 2025
@KartikLabhshetwar
Copy link
Author

hi @supalarry, @anikdhabal, @CarinaWolli is there any issue in this pr?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
authentication area: authentication, auth, google sign in, password, SAML, password reset, can't log in 🐛 bug Something isn't working community Created by Linear-GitHub Sync 🧹 Improvements Improvements to existing features. Mostly UX/UI size/S ui area: UI, frontend, button, form, input
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Email input lacks thick focus ring in Auth views (login + signup)
3 participants