Skip to content

Commit

Permalink
Update React to v18. (#7392)
Browse files Browse the repository at this point in the history
  • Loading branch information
david-yz-liu authored Jan 28, 2025
1 parent dadacb1 commit 0597f52
Show file tree
Hide file tree
Showing 72 changed files with 361 additions and 6,740 deletions.
1 change: 1 addition & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
- Implement `contain_message` and `have_message` custom Rspec matchers to check for flash message content (#7386)
- Update Python version to 3.13 in seed autotest schemas (#7388)
- Rename jupyter notebook content functions and files to generalize to html content (#7391)
- Update to React v18 (#7392)

## [v2.6.1]

Expand Down
14 changes: 7 additions & 7 deletions app/assets/javascripts/Results/context_menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,33 +14,33 @@ var annotation_context_menu = {
check_mark_annotation: {
title: "✔️",
cmd: "check_mark_annotation",
action: () => resultComponent.addQuickAnnotation("✔️"),
action: () => resultComponent.current.addQuickAnnotation("✔️"),
addClass: "emoji-annotation-context-menu-item",
},
thumbs_up_annotation: {
title: "👍",
cmd: "thumbs_up_annotation",
action: () => resultComponent.addQuickAnnotation("👍"),
action: () => resultComponent.current.addQuickAnnotation("👍"),
addClass: "emoji-annotation-context-menu-item",
},
heart_annotation: {
title: "❤️",
cmd: "heart_annotation",
action: () => resultComponent.addQuickAnnotation("❤️"),
action: () => resultComponent.current.addQuickAnnotation("❤️"),
addClass: "emoji-annotation-context-menu-item",
},
smile_annotation: {
title: "😄",
cmd: "smile_annotation",
action: () => resultComponent.addQuickAnnotation("😄️"),
action: () => resultComponent.current.addQuickAnnotation("😄️"),
addClass: "emoji-annotation-context-menu-item",
},
new_annotation: {
title: I18n.t("helpers.submit.create", {
model: I18n.t("activerecord.models.annotation.one"),
}),
cmd: "new_annotation",
action: () => resultComponent.newAnnotation(),
action: () => resultComponent.current.newAnnotation(),
disabled: true,
},
common_annotations: {
Expand All @@ -55,7 +55,7 @@ var annotation_context_menu = {
var clicked_element = ui.target[0];
var annot_id = get_annotation_id(clicked_element);
if (annot_id !== null && annot_id.length !== 0) {
resultComponent.editAnnotation(annot_id);
resultComponent.current.editAnnotation(annot_id);
}
},
disabled: true,
Expand All @@ -67,7 +67,7 @@ var annotation_context_menu = {
var clicked_element = $(ui.target)[0];
var annot_id = get_annotation_id(clicked_element);
if (annot_id !== null && annot_id.length !== 0) {
resultComponent.removeAnnotation(annot_id);
resultComponent.current.removeAnnotation(annot_id);
}
},
disabled: true,
Expand Down
4 changes: 2 additions & 2 deletions app/assets/javascripts/Results/keybinding.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +105,12 @@ Mousetrap.bind("enter", function (e) {
// Press shift+n for new annotation modal to appear
Mousetrap.bind("shift+n", () => {
if ($("#annotation_dialog:visible").length == 0) {
resultComponent.newAnnotation();
resultComponent.current.newAnnotation();
return false;
}
});

// When alt+enter is pressed, toggle fullscreen mode
Mousetrap.bind("alt+enter", () => {
resultComponent.toggleFullscreen();
resultComponent.current.toggleFullscreen();
});
1 change: 1 addition & 0 deletions app/controllers/submissions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,7 @@ def update_files
commit_success, commit_msg = commit_transaction(repo, txn)
flash_message(:success, I18n.t('flash.actions.update_files.success')) if commit_success
messages << commit_msg
head :ok
else
head :unprocessable_entity
end
Expand Down
5 changes: 3 additions & 2 deletions app/javascript/Components/Helpers/data_chart.jsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from "react";
import {render} from "react-dom";
import {createRoot} from "react-dom/client";
import {Bar} from "react-chartjs-2";

import {Chart, registerables} from "chart.js";
Expand Down Expand Up @@ -64,5 +64,6 @@ export class DataChart extends React.Component {
}

export function makeDataChart(elem, props) {
render(<DataChart {...props} />, elem);
const root = createRoot(elem);
root.render(<DataChart {...props} />);
}
7 changes: 5 additions & 2 deletions app/javascript/Components/Modals/submit_view_token_modal.jsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from "react";
import {render} from "react-dom";
import {createRoot} from "react-dom/client";
import Modal from "react-modal";

class SubmitViewTokenModal extends React.Component {
Expand Down Expand Up @@ -56,5 +56,8 @@ class SubmitViewTokenModal extends React.Component {
}

export function makeSubmitViewTokenModal(elem, props) {
return render(<SubmitViewTokenModal {...props} />, elem);
const root = createRoot(elem);
const component = React.createRef();
root.render(<SubmitViewTokenModal {...props} ref={component} />);
return component;
}
1 change: 0 additions & 1 deletion app/javascript/Components/Result/autosave_text_form.jsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import React from "react";
import {render} from "react-dom";
import {FontAwesomeIcon} from "@fortawesome/react-fontawesome";

import MarkdownEditor from "../markdown_editor";
Expand Down
1 change: 0 additions & 1 deletion app/javascript/Components/Result/marks_panel.jsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import React from "react";
import {render} from "react-dom";
import PropTypes from "prop-types";

import safe_marked from "../../common/safe_marked";
Expand Down
7 changes: 5 additions & 2 deletions app/javascript/Components/Result/result.jsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from "react";
import {render} from "react-dom";
import {createRoot} from "react-dom/client";

// TODO: This import seems to be required to automatically include the X-CSRF-TOKEN header on
// jQuery AJAX requests in this component, unlike all other pages. Requires further investigation.
Expand Down Expand Up @@ -1011,5 +1011,8 @@ class Result extends React.Component {
}

export function makeResult(elem, props) {
return render(<Result {...props} />, elem);
const root = createRoot(elem);
const component = React.createRef();
root.render(<Result {...props} ref={component} />);
return component;
}
8 changes: 4 additions & 4 deletions app/javascript/Components/Result/submission_file_panel.jsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from "react";
import ReactDOM from "react-dom";
import ReactDOM from "react-dom/client";

import {AnnotationManager} from "./annotation_manager";
import {FileViewer} from "./file_viewer";
Expand Down Expand Up @@ -99,7 +99,8 @@ export class SubmissionFilePanel extends React.Component {

// TODO: Incorporate DownloadSubmissionModal as true child of this component.
if (this.props.canDownload) {
ReactDOM.render(
const root = ReactDOM.createRoot(document.getElementById("download_dialog_body"));
root.render(
<DownloadSubmissionModal
fileData={this.props.fileData}
initialFile={selectedFile}
Expand All @@ -108,8 +109,7 @@ export class SubmissionFilePanel extends React.Component {
this.props.assignment_id,
this.props.submission_id
)}
/>,
document.getElementById("download_dialog_body")
/>
);
}
};
Expand Down
1 change: 0 additions & 1 deletion app/javascript/Components/Result/text_viewer.jsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import React from "react";
import {render} from "react-dom";
import Prism from "prismjs";

export class TextViewer extends React.PureComponent {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ describe("For the AssignmentSummaryTable's display of inactive groups", () => {
lti_deployments={[]}
/>
);
await screen.findByText("group_0002", {exact: false});
});

it("contains the correct amount of inactive groups in the hidden tooltip", () => {
Expand All @@ -79,10 +80,6 @@ describe("For the AssignmentSummaryTable's display of inactive groups", () => {
);
});

it("initially contains the active group", () => {
expect(screen.queryByText(/group_0002/)).toBeInTheDocument();
});

it("initially does not contain the inactive group", () => {
expect(screen.queryByText(/group_0001/)).not.toBeInTheDocument();
});
Expand Down
4 changes: 2 additions & 2 deletions app/javascript/Components/__tests__/binary_viewer.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ describe("BinaryViewer", () => {
expect(screen.queryByText(successfulFetchResp)).not.toBeInTheDocument();
expect(screen.getByText(I18n.t("submissions.get_anyway"))).toBeInTheDocument();
fireEvent.click(screen.getByText(I18n.t("submissions.get_anyway")));
expect(screen.getByText(successfulFetchResp)).toBeInTheDocument();
await screen.findByText(successfulFetchResp);
expect(screen.queryByText(I18n.t("submissions.get_anyway"))).not.toBeInTheDocument();
});

Expand Down Expand Up @@ -120,7 +120,7 @@ describe("BinaryViewer", () => {
await fireEvent.click(screen.getByText(I18n.t("submissions.get_anyway")));
await screen.findByText(successfulFetchResp);

expect(screen.getByText(successfulFetchResp.repeat(2))).toBeInTheDocument();
await screen.findByText(successfulFetchResp.repeat(2));
expect(screen.queryByText(successfulFetchResp)).not.toBeInTheDocument();
expect(screen.queryByText(I18n.t("submissions.get_anyway"))).not.toBeInTheDocument();
});
Expand Down
8 changes: 5 additions & 3 deletions app/javascript/Components/__tests__/graders_manager.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ jest.mock("@fortawesome/react-fontawesome", () => ({

describe("For the GradersManager's display of inactive groups", () => {
let groups_sample;
beforeEach(() => {
beforeEach(async () => {
groups_sample = [
{
_id: 15,
Expand Down Expand Up @@ -58,6 +58,8 @@ describe("For the GradersManager's display of inactive groups", () => {
}),
});
render(<GradersManager sections={{}} course_id={1} assignment_id={1} />);

await screen.findByText("group_0014"); // Wait for data to be rendered
});

it("contains the correct amount of inactive groups in the hidden tooltip", () => {
Expand All @@ -66,8 +68,8 @@ describe("For the GradersManager's display of inactive groups", () => {
);
});

it("initially contains the active group", () => {
expect(screen.getByText("group_0014")).toBeInTheDocument();
it("initially doesn't contain the inactive group", () => {
expect(screen.queryByText("group_0015")).not.toBeInTheDocument();
});

it("contains the inactive group after a single toggle", () => {
Expand Down
9 changes: 5 additions & 4 deletions app/javascript/Components/__tests__/instructor_table.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ describe("For the InstructorTable's display of instructors", () => {
throw `Could not find row for ${instructor.user_name}`;
};

beforeAll(() => {
beforeAll(async () => {
instructors_sample = [
{
id: 1,
Expand Down Expand Up @@ -63,9 +63,10 @@ describe("For the InstructorTable's display of instructors", () => {
}),
});
render(<InstructorTable course_id={1} />);
await screen.findByText("reid");
});

it("each instructor is displayed as a row of the table", () => {
it("each instructor is displayed as a row of the table", async () => {
instructors_sample.forEach(instructor => instructors_in_one_row(instructor));
});
});
Expand All @@ -85,8 +86,8 @@ describe("For the InstructorTable's display of instructors", () => {
render(<InstructorTable course_id={1} />);
});

it("No rows found is shown", () => {
expect(screen.queryByText("No rows found")).toBeTruthy();
it("No rows found is shown", async () => {
await screen.findByText("No rows found");
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ jest.mock("@fortawesome/react-fontawesome", () => ({
files: [
{
id: 1,
name: "New Starter File Group",
name: "My Starter File Group",
entry_rename: "",
use_rename: false,
files: [
Expand All @@ -32,8 +32,8 @@ jest.mock("@fortawesome/react-fontawesome", () => ({
},
],
sections: [
{section_id: 1, section_name: "LEC0101", group_id: 1, group_name: "New Starter File Group"},
{section_id: 2, section_name: "LEC0201", group_id: 1, group_name: "New Starter File Group"},
{section_id: 1, section_name: "LEC0101", group_id: 1, group_name: "My Starter File Group"},
{section_id: 2, section_name: "LEC0201", group_id: 1, group_name: "My Starter File Group"},
],
available_after_due: true,
starterfileType: "sections",
Expand All @@ -47,13 +47,14 @@ jest.mock("@fortawesome/react-fontawesome", () => ({
// so submit button is enabled (the point is to ensure this is possible)
pageInfo.form_changed = !readOnly;

beforeEach(() => {
beforeEach(async () => {
fetch.resetMocks();
fetch.mockResponseOnce(JSON.stringify(pageInfo));

container = render(
<StarterFileManager course_id={1} assignment_id={1} read_only={readOnly} />
);
await screen.findByRole("link", {name: "My Starter File Group"});
});

it(`all buttons on the page are ${readOnly ? "disabled" : "enabled"}`, () => {
Expand Down
7 changes: 4 additions & 3 deletions app/javascript/Components/__tests__/student_table.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ describe("For the StudentTable's display of students", () => {
throw `Could not find row for ${student.user_name}`;
};

beforeAll(() => {
beforeAll(async () => {
students_sample = [
{
_id: 9,
Expand Down Expand Up @@ -238,6 +238,7 @@ describe("For the StudentTable's display of students", () => {
}),
});
render(<StudentTable selection={[]} course_id={1} />);
await screen.findByText("c6scriab");
});

it("each student is displayed as a row of the table", () => {
Expand All @@ -262,8 +263,8 @@ describe("For the StudentTable's display of students", () => {
render(<StudentTable selection={[]} course_id={1} />);
});

it("No rows found is shown", () => {
expect(screen.queryByText("No rows found")).toBeTruthy();
it("No rows found is shown", async () => {
await screen.findByText("No rows found");
});
});
});
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {SubmissionFileManager} from "../submission_file_manager";

import {getAllByRole, render, screen} from "@testing-library/react";
import {getAllByRole, waitFor, render, screen} from "@testing-library/react";
import userEvent from "@testing-library/user-event";

describe("For the SubmissionFileManager", () => {
Expand Down Expand Up @@ -39,7 +39,7 @@ describe("For the SubmissionFileManager", () => {
required_files: [],
};

beforeEach(() => {
beforeEach(async () => {
// Unlike FileManager, files are stored in SubmissionFileManager's states, which are set when the component mounts
// and calls fetchData. As a result, we need to mock that fetch to return our data.
// We need to mock "twice" (i.e. two promises) because of how fetch works.
Expand All @@ -53,6 +53,8 @@ describe("For the SubmissionFileManager", () => {

// Render the component
render(<SubmissionFileManager course_id={1} assignment_id={1} />);
await screen.findByText("HelloWorld.java");
await screen.findByText("deferred-process.java");
});

describe("For the submissions managed by its FileManager child component", () => {
Expand All @@ -79,7 +81,7 @@ describe("For the SubmissionFileManager", () => {
await userEvent.click(rows[i]);

const filePreview = await screen.findByTestId("file-preview-root");
expect(filePreview.textContent).toContain(`Body ${i}`);
await waitFor(() => expect(filePreview.textContent).toContain(`Body ${i}`));
}
});
});
Expand Down
Loading

0 comments on commit 0597f52

Please sign in to comment.