Skip to content

Conversation

@visz11
Copy link

@visz11 visz11 commented Dec 24, 2025

CodeAnt-AI Description

Remove Next.js legacy link behavior and fix link/button rendering to stop type errors and preserve behavior

What Changed

  • Buttons rendered as links now use the Button component's href prop directly so links render correctly without legacy Link wrappers; data-testid and visuals are preserved
  • Clickable stepper dots, list items, login/reschedule links, and app-store navigation now use Link with direct anchor attributes (className, aria/current, data-testid) so styling and accessibility remain intact
  • Button and dropdown components no longer pass incompatible props (ref, disabled, type) to Link; button elements receive disabled, type, ref and click-prevention when loading
  • An image now uses a plain img element where appropriate and spacing classes were fixed for consistent layout

Impact

✅ Fewer build failures during TypeScript compile
✅ Clickable links keep expected styling and accessibility
✅ Disabled/loading buttons prevent accidental clicks

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

volnei and others added 10 commits December 3, 2025 12:55
…ut legacyBehavior

- Button.tsx: Only pass ref to button element, not to Link (Link manages its own anchor)
- Dropdown.tsx: Strip ref from props when using Link to avoid type incompatibility

This fixes the type errors that were causing API V1 and V2 builds to fail.

Co-Authored-By: Volnei Munhoz <volnei.munhoz@gmail.com>
Link component doesn't accept disabled or type props, so these should only be passed when rendering a button element.

Co-Authored-By: Volnei Munhoz <volnei.munhoz@gmail.com>
The passThroughProps spread was including a ref property that's incompatible with Link's expected ref type. This strips the ref when rendering a Link component.

Co-Authored-By: Volnei Munhoz <volnei.munhoz@gmail.com>
… types

The Button component uses a union type for props that can be either Link or button props. TypeScript can't narrow the union type properly when using React.createElement with a dynamic element type, so we use a type assertion to cast the props to the correct type.

Co-Authored-By: Volnei Munhoz <volnei.munhoz@gmail.com>
Co-Authored-By: Volnei Munhoz <volnei.munhoz@gmail.com>
Co-Authored-By: Volnei Munhoz <volnei.munhoz@gmail.com>
@codeant-ai
Copy link

codeant-ai bot commented Dec 24, 2025

CodeAnt AI is reviewing your PR.


Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

@gemini-code-assist
Copy link

Summary of Changes

Hello @visz11, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request modernizes the application's navigation by updating all instances of Next.js Link components to conform to the latest API, which deprecates legacyBehavior and passHref. It also refactors core UI components like Button and ButtonOrLink to intelligently render a Link when an href is present, streamlining their usage and improving consistency across the codebase. These changes ensure compatibility with newer Next.js versions and simplify future development.

Highlights

  • Next.js Link API Update: Migrates Next.js Link components from using legacyBehavior and passHref props to the newer API, where Link directly renders an <a> tag.
  • UI Component Refactoring: Enhances the Button and ButtonOrLink UI components to internally handle rendering a Next.js Link when an href prop is provided, eliminating the need for external Link wrappers.
  • Code Simplification: Reduces boilerplate by removing redundant Link imports and <span> wrappers, leading to cleaner and more maintainable JSX.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@coderabbitai
Copy link

coderabbitai bot commented Dec 24, 2025

Walkthrough

This pull request modernizes Next.js Link component usage across the codebase by removing legacy patterns and refactoring the Button component. The Button component in the UI package is refactored from a dynamic element creation approach to explicit JSX branching that handles Link and button paths separately. Multiple files—including Setup, forgot-password-single-view, bookings-single-view, and UI components—are updated to replace Link wrapped around buttons with direct Link components or Button components with href props. Legacy Link props such as passHref and legacyBehavior are removed throughout, simplifying the navigation implementation while preserving functionality.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Description check ❓ Inconclusive No pull request description was provided by the author; the changeset is missing context about the modifications. Add a detailed pull request description explaining why legacyBehavior is being removed from Next.js Link components and how these changes maintain navigation functionality.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: removing legacy behavior from Next.js Link components throughout the codebase.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/remove-links-legacybehavior

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.

@codeant-ai codeant-ai bot added the size:L This PR changes 100-499 lines, ignoring generated files label Dec 24, 2025
@refacto-visz
Copy link

refacto-visz bot commented Dec 24, 2025

Refacto PR Summary

Migrated Next.js Link components from legacy behavior to modern App Router pattern by removing legacyBehavior and passHref props. This modernization aligns with Next.js 13+ standards where Link components directly render anchor elements instead of wrapping them, improving performance and reducing DOM complexity.

Key Changes:

  • Removed legacyBehavior and passHref props from all Link components across the codebase
  • Updated Button component to handle both link and button rendering without type conflicts
  • Migrated Link usage to direct className and onClick prop passing instead of child anchor wrapping
  • Added ESLint disable comment for img elements where Next.js Image optimization isn't needed
  • Simplified dropdown and stepper components to use modern Link API

Change Highlights

Click to expand
  • packages/ui/components/button/Button.tsx: Refactored to render Link directly with proper prop handling
  • packages/ui/components/dropdown/Dropdown.tsx: Updated ButtonOrLink to use modern Link API
  • apps/web/components/apps/make/Setup.tsx: Converted Button with href prop instead of Link wrapper
  • apps/web/modules/bookings/views/bookings-single-view.tsx: Migrated reschedule/login links to direct Link usage
  • packages/ui/components/form/step/Stepper.tsx: Updated stepper navigation to use className prop on Link
  • packages/ui/components/list/List.tsx: Removed legacyBehavior from list item links
  • apps/web/modules/auth/forgot-password/[id]/forgot-password-single-view.tsx: Simplified Link structure
  • packages/app-store/_components/AppNotInstalledMessage.tsx: Updated Button to use href prop directly

Sequence Diagram

sequenceDiagram
    participant U as User
    participant C as Component
    participant L as Link Component
    participant R as Router
    
    U->>C: Click navigation element
    C->>L: Render with href + className
    L->>L: Create anchor element directly
    L->>R: Navigate to route
    R-->>U: Page transition
    
    Note over C,L: Legacy: Link wraps <a> child
    Note over C,L: Modern: Link renders <a> directly
Loading

Testing Guide

Click to expand
  1. Navigation functionality: Click all navigation links throughout the app to ensure routing still works correctly
  2. Button links: Test Button components with href props (like "Done" button in Make setup) to verify they navigate properly
  3. Styling preservation: Verify that all link styling (underlines, colors, hover states) remains intact after Link migration
  4. Dropdown navigation: Test dropdown menu items with href props to ensure they navigate and close properly
  5. Stepper navigation: Navigate through multi-step forms to confirm stepper links work correctly with new Link API

@refacto-visz
Copy link

refacto-visz bot commented Dec 24, 2025

Refacto is reviewing this PR. Please wait for the review comments to be posted.

Copy link

@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 (2)
packages/ui/components/list/List.tsx (1)

100-106: Consider removing passHref for consistency.

While ListItem was updated to remove legacy Link props, ListLinkItem still uses passHref (line 101). For consistency across the codebase and to complete the modernization effort, consider removing passHref here as well.

🔎 Suggested fix
       <Link
-        passHref
         href={href}
         className={classNames(
packages/ui/components/form/step/Stepper.tsx (1)

71-71: Consider using named export.

As per coding guidelines, prefer named exports over default exports for better tree-shaking and refactoring. The Stepper component would benefit from being a named export.

🔎 Suggested change
-export default Stepper;
+export { Stepper };

Note: This would require updating import statements in consuming files.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f350542 and 4c653c0.

📒 Files selected for processing (8)
  • apps/web/components/apps/make/Setup.tsx
  • apps/web/modules/auth/forgot-password/[id]/forgot-password-single-view.tsx
  • apps/web/modules/bookings/views/bookings-single-view.tsx
  • packages/app-store/_components/AppNotInstalledMessage.tsx
  • packages/ui/components/button/Button.tsx
  • packages/ui/components/dropdown/Dropdown.tsx
  • packages/ui/components/form/step/Stepper.tsx
  • packages/ui/components/list/List.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{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:

  • apps/web/modules/auth/forgot-password/[id]/forgot-password-single-view.tsx
  • packages/ui/components/form/step/Stepper.tsx
  • apps/web/modules/bookings/views/bookings-single-view.tsx
  • apps/web/components/apps/make/Setup.tsx
  • packages/app-store/_components/AppNotInstalledMessage.tsx
  • packages/ui/components/dropdown/Dropdown.tsx
  • packages/ui/components/button/Button.tsx
  • packages/ui/components/list/List.tsx
🧬 Code graph analysis (2)
apps/web/components/apps/make/Setup.tsx (1)
packages/ui/components/button/Button.tsx (1)
  • Button (221-361)
packages/app-store/_components/AppNotInstalledMessage.tsx (1)
packages/ui/components/button/Button.tsx (1)
  • Button (221-361)
🔇 Additional comments (13)
packages/ui/components/list/List.tsx (1)

59-62: LGTM! Link modernization is correct.

The removal of passHref and legacyBehavior aligns with Next.js 15 best practices where Link components no longer require these legacy props.

packages/ui/components/form/step/Stepper.tsx (1)

29-61: LGTM! Link modernization is correct.

The replacement of anchor-wrapped Links with direct Link components carrying className is appropriate for Next.js 15. Accessibility is properly maintained with sr-only labels, and navigation props (shallow, replace) are correctly applied.

apps/web/modules/auth/forgot-password/[id]/forgot-password-single-view.tsx (1)

48-52: LGTM! Semantically improved navigation.

The replacement of a Link-wrapped button with a styled Link component is correct for Next.js 15 and semantically more appropriate since this is a navigation action rather than a form submission or state change.

apps/web/modules/bookings/views/bookings-single-view.tsx (2)

809-816: LGTM! Link modernization is correct.

The replacement of the nested span-wrapped Link with a direct Link component carrying className and data-testid is appropriate for Next.js 15. This simplifies the DOM structure while maintaining functionality and testability.


839-848: LGTM! Link modernization is correct.

The replacement of the span-wrapped Link with a direct Link component is appropriate for Next.js 15. The styling and test attributes are correctly applied directly to the Link component.

packages/ui/components/dropdown/Dropdown.tsx (1)

144-154: LGTM! Correct ref handling for Link.

The refactoring to explicitly strip the ref when rendering a Link is correct, as Next.js Link manages its own anchor element. The explicit branching between Link and button paths is cleaner and more type-safe than dynamic element creation.

packages/app-store/_components/AppNotInstalledMessage.tsx (1)

18-21: LGTM! Consistent with Button refactor.

The migration from Link-wrapped navigation to Button with href is correct and consistent with the broader Button component refactor that supports href-based navigation. This provides a more consistent API surface for navigation actions.

apps/web/components/apps/make/Setup.tsx (2)

73-75: Verify the need for raw img element.

The eslint-disable comment suppresses the Next.js recommendation to use next/image instead of <img>. Consider whether this image could benefit from Next.js image optimization (automatic format conversion, lazy loading, responsive sizing).

If the image is from /api/app-store/make/icon.svg and requires dynamic loading or specific handling that next/image doesn't support well, this is acceptable. Otherwise, consider using next/image for better performance.


137-139: LGTM! Consistent with Button refactor.

The migration to Button with href for navigation is correct and consistent with the broader Button component refactor that now supports href-based navigation.

packages/ui/components/button/Button.tsx (4)

242-250: LGTM! Well-structured preparation logic.

The computed values for disabled state, link detection, className, and click handling are well-organized and clearly separate concerns. The explicit || false on line 242, while technically redundant, improves code clarity.


252-327: LGTM! Effective content extraction.

Extracting the button content into a reusable block reduces code duplication between the Link and Button rendering paths. The loading indicator and icon positioning logic is correctly shared.


329-341: Verify tooltip support for Link-based buttons.

The Link rendering path doesn't wrap the Link in the Wrapper component, which means buttons with href props won't display tooltips. This differs from regular buttons (lines 343-360) that are wrapped and support tooltips.

Confirm whether this is intentional. If Link-based buttons should support tooltips, consider wrapping the Link in the Wrapper component similarly to the button path.

💡 Potential fix if tooltips should be supported
   if (isLink) {
     return (
+      <Wrapper
+        data-testid="wrapper"
+        tooltip={props.tooltip}
+        tooltipSide={tooltipSide}
+        tooltipOffset={tooltipOffset}
+        tooltipClassName={tooltipClassName}>
         <Link
           {...(passThroughProps as Omit<JSX.IntrinsicElements["a"], "href" | "onClick" | "ref"> & LinkProps)}
           shallow={shallow && shallow}
           className={buttonClassName}
           onClick={handleClick}>
           {buttonContent}
         </Link>
+      </Wrapper>
     );
   }

343-360: LGTM! Correct button rendering with ref forwarding.

The button rendering path correctly forwards the ref to the underlying button element and wraps it with the Wrapper for tooltip support. Type casting is appropriately used to handle the union type.

@refacto-visz
Copy link

refacto-visz bot commented Dec 24, 2025

Code Review: Link Migration & Component Improvements

PR Confidence Score: 🟨 4 / 5

👍 Well Done
Consistent Link Migration

Systematic removal of legacyBehavior props and wrapper elements improves Next.js 13+ compatibility and reduces component complexity

Bundle Size Optimization

Direct Link component usage eliminates wrapper elements and reduces DOM tree depth for better rendering performance

📁 Selected files for review (8)
  • apps/web/components/apps/make/Setup.tsx
  • apps/web/modules/auth/forgot-password/[id]/forgot-password-single-view.tsx
  • apps/web/modules/bookings/views/bookings-single-view.tsx
  • packages/app-store/_components/AppNotInstalledMessage.tsx
  • packages/ui/components/button/Button.tsx
  • packages/ui/components/dropdown/Dropdown.tsx
  • packages/ui/components/form/step/Stepper.tsx
  • packages/ui/components/list/List.tsx
🎯 Custom Instructions
❌ Unapplied Instructions
Organization Guidelines

Reason: Your set path patterns [src/, config/] don't match any selected files for review; Your set extension patterns [.java] don't match any selected files for review

📝 Additional Comments
packages/ui/components/button/Button.tsx (3)
Navigation State Consistency

Link component receives all passThroughProps without validation potentially causing navigation state inconsistencies. Invalid props passed to Next.js Link could cause routing failures. Consider prop filtering for Link-specific attributes to ensure reliable navigation behavior.

Standards:

  • ISO-IEC-25010-Reliability-Maturity
  • ISO-IEC-25010-Functional-Correctness-Appropriateness
Context References
  • next/link - LinkProps interface from Next.js - type casting may allow invalid props
Bundle Size Reduction

Direct Link component usage eliminates wrapper elements and reduces component tree depth. This optimization removes intermediate DOM nodes and improves rendering performance by reducing React reconciliation overhead. Bundle size benefits from eliminating legacy behavior prop handling.

Standards:

  • ISO-IEC-25010-Performance-Efficiency-Resource-Utilization
  • Optimization-Pattern-Component-Tree-Reduction
Context References
  • packages/ui/components/button/Button.tsx - buttonClasses - CSS-in-JS optimization for styling efficiency
Disabled State Logic

Boolean logic includes redundant '|| false' operation that has no effect on the result. The expression 'props.disabled || loading' already produces boolean value, making the additional '|| false' unnecessary and potentially confusing for future maintainers.

Standards:

  • Boolean-Logic-Simplification
  • Code-Clarity-Standards
packages/ui/components/form/step/Stepper.tsx (1)
DOM Rendering Efficiency

Flattened Link structure eliminates nested anchor elements reducing DOM complexity. Direct className application on Link component removes wrapper divs improving rendering performance. Shallow routing with replace prevents unnecessary navigation stack growth optimizing browser memory usage.

Standards:

  • ISO-IEC-25010-Performance-Efficiency-Time-Behavior
  • Optimization-Pattern-DOM-Flattening
apps/web/components/apps/make/Setup.tsx (1)
Button Component Consistency

Migration from Link wrapper to Button href prop improves component API consistency. The Button component now handles navigation internally rather than requiring external Link wrapping. This reduces component composition complexity and provides cleaner API surface.

Standards:

  • Clean-Code-Functions
  • Design-Pattern-Component
Context References
  • packages/ui/components/button/Button.tsx - Button component supports href prop for navigation
apps/web/modules/auth/forgot-password/[id]/forgot-password-single-view.tsx (1)
Link Simplification Pattern

Removal of passHref and legacyBehavior props simplifies Link usage following Next.js 13+ patterns. Direct className application to Link component eliminates unnecessary button wrapper, reducing DOM complexity while maintaining identical styling and functionality.

Standards:

  • Clean-Code-Simplicity
  • Maintainability-Quality-Coupling
🧰 Additional context used
packages/ui/components/button/Button.tsx (1)
packages/ui/components/button/Button.tsx (1)
  • buttonClasses (42-219)

<img className="h-11" src="/api/app-store/make/icon.svg" alt="Make Logo" />
</div>
<div className="ml-2 ltr:mr-2 rtl:ml-2 md:ml-5">
<div className="ml-2 md:ml-5 ltr:mr-2 rtl:ml-2">
Copy link

Choose a reason for hiding this comment

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

CSS Class Ordering

CSS class ordering places responsive modifier 'md:ml-5' between base 'ml-2' and directional classes 'ltr:mr-2 rtl:ml-2'. This creates potential cascade conflicts where directional classes may override responsive spacing. Consistent ordering should group base, responsive, then directional modifiers.

Standards
  • CSS-Cascade-Logic
  • Responsive-Design-Consistency

Comment on lines +246 to +250
const handleClick = disabled
? (e: React.MouseEvent<HTMLElement, MouseEvent>) => {
e.preventDefault();
}
: props.onClick;
Copy link

Choose a reason for hiding this comment

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

Missing Error Handling

Button component passes through onClick handler without error boundary protection. Navigation failures or handler exceptions can propagate uncaught causing component crashes. User interactions become unreliable when handler throws unexpected errors.

Standards
  • ISO-IEC-25010-Reliability-Fault-Tolerance
  • ISO-IEC-25010-Functional-Correctness-Appropriateness
Context References
  • packages/ui/components/button/Button.tsx - Button component handles both link and button elements - no error handling wrapper

`/booking/${bookingInfo?.uid}`
)}`}
className="underline"
data-testid="reschedule-link">
Copy link

Choose a reason for hiding this comment

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

Suggestion: The login link for users who must log in before updating a booking is tagged with the reschedule-link test id, which can cause automated tests or tooling that target the reschedule action to mistakenly interact with this login link instead, leading to incorrect flows being exercised in scenarios where login is required to update a booking. [possible bug]

Severity Level: Critical 🚨

Suggested change
data-testid="reschedule-link">
data-testid="login-link">
Why it matters? ⭐

The current code places data-testid="reschedule-link" on a login link used when requiresLoginToUpdate is true.
That re-uses a test id that's also used for the actual reschedule action elsewhere and can cause automated tests or tooling to
target the wrong element. Renaming the test id to something specific to the login action (e.g. "login-link") fixes an actual
test/selector collision rather than being a purely cosmetic change.

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** apps/web/modules/bookings/views/bookings-single-view.tsx
**Line:** 814:814
**Comment:**
	*Possible Bug: The login link for users who must log in before updating a booking is tagged with the `reschedule-link` test id, which can cause automated tests or tooling that target the reschedule action to mistakenly interact with this login link instead, leading to incorrect flows being exercised in scenarios where login is required to update a booking.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.

Comment on lines +839 to +846
<Link
href={`/reschedule/${seatReferenceUid || bookingInfo?.uid}${
currentUserEmail
? `?rescheduledBy=${encodeURIComponent(currentUserEmail)}`
: ""
}`}
className="underline"
data-testid="reschedule-link">
Copy link

Choose a reason for hiding this comment

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

Suggestion: The reschedule link now has the data-testid="reschedule-link" on the <Link> element instead of the wrapping <span>, which breaks existing selectors like span[data-testid="reschedule-link"] > a used in e2e tests (e.g. booking-limits.e2e.ts) and prevents those tests from finding the reschedule URL, so moving the test id back to the span restores the DOM contract without changing user-facing behavior. [possible bug]

Severity Level: Critical 🚨

Suggested change
<Link
href={`/reschedule/${seatReferenceUid || bookingInfo?.uid}${
currentUserEmail
? `?rescheduledBy=${encodeURIComponent(currentUserEmail)}`
: ""
}`}
className="underline"
data-testid="reschedule-link">
<span className="text-default inline" data-testid="reschedule-link">
<Link
href={`/reschedule/${seatReferenceUid || bookingInfo?.uid}${
currentUserEmail
? `?rescheduledBy=${encodeURIComponent(currentUserEmail)}`
: ""
}`}
className="underline">
Why it matters? ⭐

The change in the PR moved data-testid="reschedule-link" from the wrapping span to the Link element. If existing e2e or DOM-based tests expect the test id on the wrapper span (selectors like span[data-testid="reschedule-link"] > a), the PR will break those tests.
Moving the test id back to the span preserves the prior DOM contract without affecting user-facing behavior, so this is a practical, low-risk fix to maintain test stability.

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** apps/web/modules/bookings/views/bookings-single-view.tsx
**Line:** 839:846
**Comment:**
	*Possible Bug: The reschedule link now has the `data-testid="reschedule-link"` on the `<Link>` element instead of the wrapping `<span>`, which breaks existing selectors like `span[data-testid="reschedule-link"] > a` used in e2e tests (e.g. `booking-limits.e2e.ts`) and prevents those tests from finding the reschedule URL, so moving the test id back to the span restores the DOM contract without changing user-facing behavior.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.

<p className="text-sm font-medium">{stepLabel(props.step, steps.length)}</p>
<ol role="list" className="ml-8 flex items-center space-x-5" ref={stepperRef}>
{steps.map((mapStep, index) => (
<li key={mapStep.title}>
Copy link

Choose a reason for hiding this comment

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

Suggestion: Using mapStep.title alone as the React key for each step assumes titles are globally unique; if two steps share the same title, React's reconciliation can mis-associate DOM nodes and lead to incorrect UI updates when steps change, so the key should be made unique (for example by incorporating the index). [possible bug]

Severity Level: Critical 🚨

Suggested change
<li key={mapStep.title}>
<li key={`${mapStep.title}-${index}`}>
Why it matters? ⭐

Using only mapStep.title as the key can lead to duplicated keys if two steps share a title, which breaks React reconciliation and can cause UI bugs.
Appending the index (or, better, using a stable unique id on the step) makes the key unique and prevents accidental DOM reuse. This is a practical fix for a real potential bug.

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** packages/ui/components/form/step/Stepper.tsx
**Line:** 28:28
**Comment:**
	*Possible Bug: Using `mapStep.title` alone as the React `key` for each step assumes titles are globally unique; if two steps share the same title, React's reconciliation can mis-associate DOM nodes and lead to incorrect UI updates when steps change, so the key should be made unique (for example by incorporating the index).

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.

Comment on lines +31 to +55
href={props.disableSteps ? "#" : `${href}?step=${index + 1}`}
shallow
replace
className="hover:bg-inverted block h-2.5 w-2.5 rounded-full bg-gray-600">
<span className="sr-only">{mapStep.title}</span>
</Link>
) : index + 1 === props.step ? (
<Link
href={props.disableSteps ? "#" : `${href}?step=${index + 1}`}
shallow
replace
className="relative flex items-center justify-center"
aria-current="step">
<span className="absolute flex h-5 w-5 p-px" aria-hidden="true">
<span className="bg-emphasis h-full w-full rounded-full" />
</span>
<span
className="relative block h-2.5 w-2.5 rounded-full bg-gray-600"
aria-hidden="true"
/>
<span className="sr-only">{mapStep.title}</span>
</Link>
) : (
<Link
href={props.disableSteps ? "#" : `${href}?step=${index + 1}`}
Copy link

Choose a reason for hiding this comment

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

Suggestion: Building the step URLs by blindly appending ?step=${index + 1} to the href string will break when the base href already contains query parameters (e.g. /auth/setup?mode=admin), resulting in invalid URLs like /auth/setup?mode=admin?step=2 and incorrect routing behavior; you should append step with & when a ? already exists. [logic error]

Severity Level: Minor ⚠️

Suggested change
href={props.disableSteps ? "#" : `${href}?step=${index + 1}`}
shallow
replace
className="hover:bg-inverted block h-2.5 w-2.5 rounded-full bg-gray-600">
<span className="sr-only">{mapStep.title}</span>
</Link>
) : index + 1 === props.step ? (
<Link
href={props.disableSteps ? "#" : `${href}?step=${index + 1}`}
shallow
replace
className="relative flex items-center justify-center"
aria-current="step">
<span className="absolute flex h-5 w-5 p-px" aria-hidden="true">
<span className="bg-emphasis h-full w-full rounded-full" />
</span>
<span
className="relative block h-2.5 w-2.5 rounded-full bg-gray-600"
aria-hidden="true"
/>
<span className="sr-only">{mapStep.title}</span>
</Link>
) : (
<Link
href={props.disableSteps ? "#" : `${href}?step=${index + 1}`}
href={
props.disableSteps
? "#"
: `${href}${href.includes("?") ? "&" : "?"}step=${index + 1}`
}
shallow
replace
className="hover:bg-inverted block h-2.5 w-2.5 rounded-full bg-gray-600">
<span className="sr-only">{mapStep.title}</span>
</Link>
) : index + 1 === props.step ? (
<Link
href={
props.disableSteps
? "#"
: `${href}${href.includes("?") ? "&" : "?"}step=${index + 1}`
}
shallow
replace
className="relative flex items-center justify-center"
aria-current="step">
<span className="absolute flex h-5 w-5 p-px" aria-hidden="true">
<span className="bg-emphasis h-full w-full rounded-full" />
</span>
<span
className="relative block h-2.5 w-2.5 rounded-full bg-gray-600"
aria-hidden="true"
/>
<span className="sr-only">{mapStep.title}</span>
</Link>
) : (
<Link
href={
props.disableSteps
? "#"
: `${href}${href.includes("?") ? "&" : "?"}step=${index + 1}`
}
Why it matters? ⭐

The claim is correct: blindly appending ?step=... will create malformed URLs when href already has query params.
This leads to incorrect links and potential routing issues. The improvement (conditionally using & when href contains ?) fixes a real logic bug.
Preferably this should use a proper URL/query builder (URL or next/link's object form) to avoid edge cases, but the suggested patch is a valid, minimal fix for the reported problem.

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** packages/ui/components/form/step/Stepper.tsx
**Line:** 31:55
**Comment:**
	*Logic Error: Building the step URLs by blindly appending `?step=${index + 1}` to the `href` string will break when the base `href` already contains query parameters (e.g. `/auth/setup?mode=admin`), resulting in invalid URLs like `/auth/setup?mode=admin?step=2` and incorrect routing behavior; you should append `step` with `&` when a `?` already exists.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.

@codeant-ai
Copy link

codeant-ai bot commented Dec 24, 2025

CodeAnt AI finished reviewing your PR.

@codeant-ai
Copy link

codeant-ai bot commented Dec 24, 2025

💡 Enhance Your PR Reviews

We noticed that 3 feature(s) are not configured for this repository. Enabling these features can help improve your code quality and workflow:

🚦 Quality Gates

Status: Quality Gates are not enabled at the organization level
Learn more about Quality Gates

🎫 Jira Ticket Compliance

Status: Jira credentials file not found. Please configure Jira integration in your settings
Learn more about Jira Integration

⚙️ Custom Rules

Status: No custom rules configured. Add rules via organization settings or .codeant/review.json in your repository
Learn more about Custom Rules


Want to enable these features? Contact your organization admin or check our documentation for setup instructions.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request focuses on updating the usage of next/link components across the codebase, primarily by removing the deprecated legacyBehavior and passHref props. The Button and ButtonOrLink components in the UI library have been refactored to internally handle the rendering of next/link when an href prop is provided, aligning with the modern Next.js API. This is a good modernization effort that simplifies link handling in various parts of the application.

<div className="bg-default m-auto max-w-[43em] overflow-auto rounded pb-10 md:p-10">
<div className="md:flex md:flex-row">
<div className="invisible md:visible">
{/* eslint-disable @next/next/no-img-element */}

Choose a reason for hiding this comment

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

medium

It's generally better to address the underlying ESLint rule rather than disabling it. While this might be a quick fix, consider if there's a more permanent solution to allow img elements without the alt attribute or if the alt attribute can be dynamically provided.

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

Labels

size:L This PR changes 100-499 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants