Skip to content
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

Get semantic-ui theme on react 17 and dts-cli #3083

Merged

Conversation

heath-freenome
Copy link
Member

@heath-freenome heath-freenome commented Sep 2, 2022

Reasons for making this change

  • Updated the package*.json to update to react 17 and switch from tsdx to dts-cli
  • Ran cs-format over semantic-ui after upgrading to dts-cli that uses a newer version of prettier
  • Updated the tests to mock the Ref component from @fluentui/react-component-ref because it causes tests to fail
  • Added the same additional Array tests to other themes that were added in Fix(@rjsf/antd): Check for errors.length #2576
  • Updated the 5.x migration guide and the CHANGELOG.md

Checklist

  • I'm updating documentation
  • I'm adding or updating code
    • I've added and/or updated tests. I've run npm run test:update to update snapshots, if needed.
    • I've updated docs if needed
    • I've updated the changelog with a description of the PR
  • I'm adding a new feature
    • I've updated the playground with an example use of the feature

- Updated the `package*.json` to update to react 17 and switch from `tsdx` to `dts-cli`
- Updated the tests to mock the `Ref` component from `@fluentui/react-component-ref` because it causes tests to fail
@@ -4,6 +4,12 @@ import renderer from "react-test-renderer";

import Form from "../src/index";

/** Mock the `react-component-ref` component used by semantic-ui to simply render the children, otherwise tests fail */
jest.mock("@fluentui/react-component-ref", () => ({
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be semantic-ui

Copy link
Member Author

@heath-freenome heath-freenome Sep 3, 2022

Choose a reason for hiding this comment

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

@jacqueswho Actually, it needs to be @fluentui/react-component-ref... Looking at the semantic-ui-react source code you can see that it is what needs to be mocked

@@ -4,6 +4,12 @@ import renderer from "react-test-renderer";

import Form from "../src/index";

/** Mock the `react-component-ref` component used by semantic-ui to simply render the children, otherwise tests fail */
jest.mock("@fluentui/react-component-ref", () => ({
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be semantic-ui

@@ -4,6 +4,12 @@ import renderer from "react-test-renderer";

import Form from "../src/index";

/** Mock the `react-component-ref` component used by semantic-ui to simply render the children, otherwise tests fail */
jest.mock("@fluentui/react-component-ref", () => ({
...jest.requireActual("@fluentui/react-component-ref"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be semantic-ui

@@ -4,6 +4,12 @@ import renderer from "react-test-renderer";

import Form from "../src/index";

/** Mock the `react-component-ref` component used by semantic-ui to simply render the children, otherwise tests fail */
jest.mock("@fluentui/react-component-ref", () => ({
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be semantic-ui

@heath-freenome heath-freenome merged commit 206e5b6 into rjsf-team:master Sep 3, 2022
@heath-freenome heath-freenome deleted the fix-semantic-ui-to-use-dts-cli branch September 3, 2022 07:34
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