Skip to content

Upgrade to react 19#248

Draft
badiuoanaalexandra wants to merge 3 commits intomainfrom
upgrade-to-19
Draft

Upgrade to react 19#248
badiuoanaalexandra wants to merge 3 commits intomainfrom
upgrade-to-19

Conversation

@badiuoanaalexandra
Copy link
Contributor

No description provided.

@@ -0,0 +1,30 @@
import React, { ReactNode, FormEvent, KeyboardEvent } from 'react';

Copy link
Contributor Author

@badiuoanaalexandra badiuoanaalexandra Mar 17, 2026

Choose a reason for hiding this comment

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

Temporary added until recat-submittable is upgraded

// Following the instructions provided by Radix on handling disabled
// button elements: Since disabled buttons don't fire events, you need to:
// - Render the Trigger as `span`.
// @ts-expect-error - child.props.disabled may exist on button elements
Copy link
Member

Choose a reason for hiding this comment

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

I'm not inferring from the comment why is this necessary?


test('render border', () => {
render(<TabsWithBorder />);
expect(screen.getByTestId('tabs-wrapper')).toHaveClass('border-b border--gray-light mb-neg1');
Copy link
Member

Choose a reason for hiding this comment

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

Why was mb-neg1 removed 🤔

await userEvent.hover(screen.getByRole('slider'));
const { container } = render(<ControlRange {...props} />);
const thumb = screen.getByRole('slider');
fireEvent.mouseOver(thumb);
Copy link
Member

Choose a reason for hiding this comment

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

Hmm this might impact tests upstream if userEvent had previously worked

children: ReactNode;
}

export default function Submittable({ onEnter, onCancel, children, ...rest }: SubmittableProps) {
Copy link
Member

Choose a reason for hiding this comment

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

Probably fine and likely what's in the original package but ...rest seems a little insecure?


await waitFor(() => {
expect(screen.getByRole('dialog')).toBeInTheDocument();
expect(screen.getByRole('dialog', { hidden: true })).toBeInTheDocument();
Copy link
Member

Choose a reason for hiding this comment

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

This hidden: true stuff seems suspicious

Copy link
Member

Choose a reason for hiding this comment

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

Not seeing any issues running this locally and testing it out at least!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants