Skip to content

Conversation

@KartikLabhshetwar
Copy link
Contributor

@KartikLabhshetwar KartikLabhshetwar commented Oct 25, 2025

What does this PR do?

Visual Demo (For contributors especially)

Image Demo (if applicable):

before:
Screenshot 2025-10-25 at 3 46 20 PM

after:
no error visible

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

Summary by cubic

Removed deprecated Next.js Link props (legacyBehavior, passHref) and updated components to the current Link API. This resolves deprecation warnings (CAL-6638, #24685) without changing UI or behavior.

  • Refactors
    • Replaced legacy Link usage in Make/Zapier Setup, Forgot Password, Bookings, and AppNotInstalledMessage.
    • Updated Button, Dropdown, and List to pass className/onClick to Link and avoid nested anchor tags.
    • Adjusted Stepper to render non-anchor children inside Link while preserving shallow/replace.

@vercel
Copy link

vercel bot commented Oct 25, 2025

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

A member of the Team first needs to authorize it.

@github-actions github-actions bot added the 🐛 bug Something isn't working label Oct 25, 2025
@KartikLabhshetwar KartikLabhshetwar marked this pull request as ready for review October 25, 2025 10:22
@KartikLabhshetwar KartikLabhshetwar requested review from a team as code owners October 25, 2025 10:22
@graphite-app graphite-app bot added the community Created by Linear-GitHub Sync label Oct 25, 2025
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

8 issues found across 9 files

Prompt for AI agents (all 8 issues)

Understand the root cause of the following 8 issues and fix them.


<file name="packages/ui/components/list/List.tsx">

<violation number="1" location="packages/ui/components/list/List.tsx:54">
Link items no longer receive their onClick/ARIA/data props because only className and data-testid are passed to &lt;Link/&gt;. Please spread the accumulated props onto the Link to preserve existing behavior.</violation>
</file>

<file name="apps/web/modules/auth/forgot-password/[id]/forgot-password-single-view.tsx">

<violation number="1" location="apps/web/modules/auth/forgot-password/[id]/forgot-password-single-view.tsx:48">
Dropping legacyBehavior means Link now renders an anchor, so this button becomes nested inside &lt;a&gt;, which is invalid HTML and breaks accessibility. Render the link content as an anchor (or refactor to avoid nested interactive controls).</violation>
</file>

<file name="apps/web/components/apps/zapier/Setup.tsx">

<violation number="1" location="apps/web/components/apps/zapier/Setup.tsx:141">
The new Link API wraps children in an anchor, so this now produces `&lt;a&gt;&lt;button&gt;…&lt;/button&gt;&lt;/a&gt;`. The Button component doesn’t pick up `href`, leaving the inner element as `&lt;button&gt;`, which violates HTML semantics and accessibility. Please ensure the child renders the anchor itself (e.g., pass `href` to Button or restore the legacy behavior).</violation>
</file>

<file name="apps/web/components/apps/make/Setup.tsx">

<violation number="1" location="apps/web/components/apps/make/Setup.tsx:137">
Dropping legacyBehavior means Link now renders an &lt;a&gt;, so this change nests our Button’s &lt;button&gt; inside an anchor. That’s invalid/interactive markup and causes accessibility issues; convert the styling to use Button `asChild` (or otherwise avoid wrapping a &lt;button&gt; in &lt;a&gt;).</violation>
</file>

<file name="packages/ui/components/button/Button.tsx">

<violation number="1" location="packages/ui/components/button/Button.tsx:336">
forwardRef is now ignored when href is provided, so callers cannot obtain the anchor/button element through the ref in link mode. Please pass forwardedRef down to the Link component to keep ref support intact.</violation>

<violation number="2" location="packages/ui/components/button/Button.tsx:340">
The Link branch no longer forwards pass-through props (target, rel, aria-*, data-*) to the rendered anchor, so consumers lose those attributes compared with the previous implementation. Please spread the elementProps onto the Link to preserve existing behavior.</violation>
</file>

<file name="packages/ui/components/form/step/Stepper.tsx">

<violation number="1" location="packages/ui/components/form/step/Stepper.tsx:35">
Moving `aria-current` from the anchor to the inner span removes the current-step announcement for assistive tech. Please keep `aria-current=&quot;step&quot;` on the Link element so the active step remains accessible.</violation>
</file>

<file name="packages/app-store/_components/AppNotInstalledMessage.tsx">

<violation number="1" location="packages/app-store/_components/AppNotInstalledMessage.tsx:20">
Dropping legacyBehavior here makes Link render an &lt;a&gt; around Button, but Button (without an href prop) still creates a real &lt;button&gt;. That gives an &lt;a&gt;&lt;button&gt;…&lt;/button&gt;&lt;/a&gt; nesting, which React flags and can break navigation/accessibility. Pass the href directly to Button instead so it renders the Link internally.</violation>
</file>

Since this is your first cubic review, here's how it works:

  • cubic automatically reviews your code and comments on bugs and improvements
  • Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
  • Ask questions if you need clarification on any suggestion

React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.

@volnei volnei marked this pull request as draft October 27, 2025 12:13
Copy link
Contributor

@volnei volnei left a comment

Choose a reason for hiding this comment

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

@KartikLabhshetwar, thank you for your contribution! There's some cubic comments that needs to be addressed.. do you want to move with them?

@KartikLabhshetwar
Copy link
Contributor Author

@KartikLabhshetwar, thank you for your contribution! There's some cubic comments that needs to be addressed.. do you want to move with them?

I am working on it.

@KartikLabhshetwar KartikLabhshetwar marked this pull request as ready for review October 27, 2025 13:09
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 9 files

@AshutoshDevgotra
Copy link

SHOULD I FIX THIS?

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 9 files

Prompt for AI agents (all 1 issues)

Understand the root cause of the following 1 issues and fix them.


<file name="packages/ui/components/button/Button.tsx">

<violation number="1" location="packages/ui/components/button/Button.tsx:343">
Forward the computed disabled flag to the Link so link-style buttons still receive the `[disabled]` attribute (required for the existing disabled:* styling when loading sets disabled implicitly).</violation>
</file>

React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.

href={props.href}
shallow={shallow && shallow}
className={elementProps.className}
onClick={elementProps.onClick}>
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Oct 28, 2025

Choose a reason for hiding this comment

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

Forward the computed disabled flag to the Link so link-style buttons still receive the [disabled] attribute (required for the existing disabled:* styling when loading sets disabled implicitly).

Prompt for AI agents
Address the following comment on packages/ui/components/button/Button.tsx at line 343:

<comment>Forward the computed disabled flag to the Link so link-style buttons still receive the `[disabled]` attribute (required for the existing disabled:* styling when loading sets disabled implicitly).</comment>

<file context>
@@ -333,8 +333,15 @@ export const Button = forwardRef&lt;HTMLAnchorElement | HTMLButtonElement, ButtonPr
+      href={props.href}
+      shallow={shallow &amp;&amp; shallow}
+      className={elementProps.className}
+      onClick={elementProps.onClick}&gt;
+      {elementContent}
     &lt;/Link&gt;
</file context>
Fix with Cubic

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 3 files

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐛 bug Something isn't working community Created by Linear-GitHub Sync size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Next.js Link Component Deprecation Warnings

3 participants