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.

Sign up for GitHub

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

fix: debug and compare modes fix #390

Merged
merged 6 commits into from
Dec 17, 2024
+23 −3
Merged

fix: debug and compare modes fix #390

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
917e34e
crash fixed
dariaterekhova-actionengine Dec 6, 2024
22ebd78
two file uplosders fix
dariaterekhova-actionengine Dec 9, 2024
c6f3027
Tests fix
dariaterekhova-actionengine Dec 9, 2024
435f8ac
review fix
dariaterekhova-actionengine Dec 12, 2024
0064a62
lint fix
dariaterekhova-actionengine Dec 12, 2024
a13e8df
Merge commit 'beb3721ece430e3875e5c56a3f20478fcfa129f9' into dt/debug…
dariaterekhova-actionengine Dec 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/components/bookmarks-panel/bookmarks-panel.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ jest.mock("../../utils/hooks/layout");

const dragAndDropText = "Drag and drop your json file here";

Object.defineProperty(globalThis, "crypto", {
value: {
randomUUID: () => "",
},
});

const TEST_BOOKMARKS = [
{
id: "testId1",
Expand Down
1 change: 1 addition & 0 deletions src/components/comparison/comparison-side/comparison-side.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,7 @@ export const ComparisonSide = ({
.map((sublayer) => ({
id: sublayer.id,
url: sublayer.url,
fetch: sublayer.fetch,
token: sublayer.token,
type: sublayer.type ?? TilesetType.I3S,
}));
Expand Down
6 changes: 6 additions & 0 deletions src/components/layers-panel/insert-panel/insert-panel.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ import "@testing-library/jest-dom";
const onInsertMock = jest.fn();
const onCancelMock = jest.fn();

Object.defineProperty(globalThis, "crypto", {
value: {
randomUUID: () => "",
},
});

const callRender = (
renderFunc,
props = {},
Expand Down
6 changes: 6 additions & 0 deletions src/components/upload-panel/upload-panel.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@ jest.mock("@hyperjump/json-schema", () => ({
),
}));

Object.defineProperty(globalThis, "crypto", {
value: {
randomUUID: () => "",
},
});

const onCancel = jest.fn();
const onFileUploaded = jest.fn();

Expand Down
6 changes: 3 additions & 3 deletions src/components/upload-panel/upload-panel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,9 @@ import { UploadPanelItem } from "./upload-panel-item";

import UploadIcon from "../../../public/icons/upload.svg";
import { Layout } from "../../utils/enums";
import { useRef, useState } from "react";
import { useMemo, useRef, useState } from "react";
import { useAppLayout } from "../../utils/hooks/layout";

const UPLOAD_INPUT_ID = "upload-file-input";

const FileInteractionContainer = styled.label`
box-sizing: border-box;
display: flex;
Expand Down Expand Up @@ -83,6 +81,8 @@ export const UploadPanel = ({
onFileUploaded,
onFileEvent,
}: UploadProps) => {
const UPLOAD_INPUT_ID = useMemo(() => `upload-file-input${crypto.randomUUID()}`, []);
Copy link
Collaborator

Choose a reason for hiding this comment

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

What does this fix? It would be good to have a list of changes in the PR description with screenshots when applicable

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If you have several upload panels open at the same time (for example in comparison mode) all actions go to the first one and you're unable to choose a file on the second one. This happens because all file inputs and divs that catch the actions have the same id. This fix generates the unique id for every upload panel so actions will be passed inside every upload panel separately

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't understand how randomUUID: () => "" can return something unique

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The stab needs to be there because jsdom doesn't have a stub for browser's crypto. And since we don't have tests where two upload panels would be shown at the same time, so randomUUID: () => "" is enough for all our tests.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we avoid hacks?
The offered approach is very weird:

  • We make a global variable that is bad practice;
  • We declare randomUUID that doesn't return a UUID;
  • Why crypto.reandomUUID? Is there any reason to mock nodejs API?

For example, we can use Redux to generate unique application-wide integer ids.


const layout = useAppLayout();
const [dragActive, setDragActive] = useState(false);
const [fileUploaded, setFileUploaded] = useState("");
Expand Down
1 change: 1 addition & 0 deletions src/pages/debug-app/debug-app.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ export const DebugApp = () => {
.map((sublayer) => ({
id: sublayer.id,
url: sublayer.url,
fetch: sublayer.fetch,
token: sublayer.token,
type: sublayer.type ?? TilesetType.I3S,
}));
Expand Down
Loading