uiwjs / react-codemirror

CodeMirror 6 component for React. @codemirror https://uiwjs.github.io/react-codemirror/
https://uiwjs.github.io/react-codemirror/
MIT License
1.65k stars 132 forks source link

Duplicate mergeView elements #517

Open kimfucious opened 1 year ago

kimfucious commented 1 year ago

I'm trying to adjust the height of a CodeMirrorMerge.

In reading about how to do this, it seems that the recommended practice is to modify the .cm-mergeView CSS class.

I did this and noticed that there are multiple <div> elements that seem to be duplicate instances.

In the below, the highlighted portion represents one of the two instances, and it's empty with a height of ~30px.

The second instance shows what is expected with the text that needs diffing.

One problem around this is that adjusting the height on the .cm-mergeView class adjusts the heights on both elements, but the real problem seems to be that there are duplicates.

I can't think of anywhere in my code that is creating two codeMerges, but I could very well be doing something wrong.

Any help toward figuring out this issue would be most appreciated.

image

image

import { useContext, useMemo, useState } from "react";
import { ActionType } from "../../../types"; // enum
import { Button, Spinner } from "react-bootstrap";
import { EditorState } from "@codemirror/state";
import { EditorView } from "codemirror";
import { markdown } from "@codemirror/lang-markdown";
import { materialDarkInit } from "@uiw/codemirror-theme-material";
import { restoreScene } from "../../../actions";
import { solarizedLightInit } from "@uiw/codemirror-theme-solarized";
import { ThemeContext } from "../../../contexts/ThemeProvider";
import { useAppDispatch, useAppSelector } from "../../../hooks/reduxHooks";
import { vim } from "@replit/codemirror-vim";
import CodeMirrorMerge from "react-codemirror-merge";
import config from "../../../config/config.json";

export default function MergeEditor() {
    const Original = CodeMirrorMerge.Original;
    const Modified = CodeMirrorMerge.Modified;
    const { app } = useAppSelector((state) => state);
    const dispatch = useAppDispatch();
    const { isDark } = useContext(ThemeContext);
    const [changedText, setChangedText] = useState("");
    const [spinning, setSpinning] = useState(false);

    const { abandonedScene, original, modified } = useMemo(() => {
        let modified = "";
        const abandonedScene = app.scenes.find(
            (item) => item.id === app.abandonedSceneId
        );
        const original = abandonedScene?.text;
        const draftInStorage = localStorage.getItem(config.DRAFT_SCENE_KEY);
        if (draftInStorage) {
            const unsavedScene = JSON.parse(draftInStorage);
            if (abandonedScene?.id === unsavedScene?.id) {
                modified = unsavedScene.text.trim();
            }
        }
        return { abandonedScene, original, modified };
    }, [app.abandonedSceneId, app.scenes]);

    // const height = useMemo(() => {
    //     return `calc(100vh - ${sizes.totalHeightOffset + 32}px)`;
    // }, [sizes.totalHeightOffset]);

    function handleChange(str: string) {
        setChangedText(str);
    }

    function handleClose() {
        dispatch({
            type: ActionType.SET_MERGE_EDITOR,
            payload: { isDiffingAbandonedScene: false },
        });
    }

    function handleAbandon() {
        localStorage.removeItem(config.DRAFT_SCENE_KEY);
        handleClose();
    }

    async function handleRestore() {
        try {
            setSpinning(true);
            if (!abandonedScene) {
                throw new Error("No abandoned scene!");
            }
            await dispatch(
                restoreScene({ ...abandonedScene, text: changedText })
            );
            setSpinning(false);
            localStorage.removeItem(config.DRAFT_SCENE_KEY);
            handleClose();
        } catch (error) {
            setSpinning(false);
        }
    }
    const { defaultThemeOptions, theme } = useMemo(() => {
        const defaultThemeOptions = EditorView.theme(
            {
                "&": {
                    background: isDark
                        ? "#2e3235 !important"
                        : "#2e3235 !important",
                    backgroundColor: isDark
                        ? "#2e3235 !important"
                        : "#fdf6e3 !important",
                    foreground: isDark
                        ? "#bdbdbd !important"
                        : "#657b83 !important",
                    caret: isDark ? "#a0a4ae !important" : "#586e75 !important",
                    selection: isDark
                        ? "#d7d4f0 !important"
                        : "#dfd9c8 !important",
                    selectionMatch: isDark
                        ? "#d7d4f0 !important"
                        : "#dfd9c8 !important",
                    gutterBackground: isDark
                        ? "#2e3235 !important"
                        : "#00000010 !important",
                    gutterActiveBackground: isDark
                        ? "#4f5b66 !important"
                        : "#00000010 !important",
                    gutterActiveForeground: isDark
                        ? "#000 !important"
                        : "#657b83 !important",
                    gutterForeground: isDark
                        ? // ? "#999 !important"
                          "#ff69b4 !important"
                        : "#657b83 !important",
                    lineHighlight: isDark
                        ? "#545b61 !important"
                        : "#dfd9c8 !important",
                },
            },
            {
                dark: isDark,
            }
        );
        const theme = isDark
            ? materialDarkInit({ theme: "dark" })
            : solarizedLightInit({ theme: "light" });
        return { defaultThemeOptions, theme };
    }, [isDark]);

    return (
        <div className="container d-flex flex-column align-items-center w-100">
            <CodeMirrorMerge
                className="w-100"
                gutter={false}
                highlightChanges
                orientation="a-b"
                revertControls="b-to-a"
            >
                <Original
                    extensions={[
                        vim({ status: false }),
                        defaultThemeOptions,
                        theme,
                        markdown(),
                        EditorView.lineWrapping,
                        EditorView.contentAttributes.of({ spellcheck: "true" }),
                    ]}
                    value={original}
                    onChange={(str) => handleChange(str)}
                />
                <Modified
                    value={modified}
                    extensions={[
                        vim({ status: false }),
                        defaultThemeOptions,
                        theme,
                        markdown(),
                        EditorView.lineWrapping,
                        EditorView.editable.of(false),
                        EditorState.readOnly.of(true),
                    ]}
                />
            </CodeMirrorMerge>

            <div className="d-flex justify-content-between w-100">
                <small>Current Scene</small>
                <small>Unsaved Changes (read only)</small>
            </div>
            {changedText !== modified && (
                <div className="d-flex justify-content-center mt-3 w-100">
                    You can merge the unsaved changes by clicking the little
                    arrow
                </div>
            )}
            <div className="d-flex align-items-center justify-content-between my-5 w-100">
                <Button
                    className="d-flex align-items-center justify-content-center"
                    onClick={() => handleAbandon()}
                    style={{ width: 200 }}
                    variant="danger"
                >
                    <i className="bi bi-trash" />
                    <span className="ms-2">Abandon Changes</span>
                </Button>
                <div className="d-flex align-items-center">
                    <Button
                        className="d-flex align-items-center justify-content-center ms-3"
                        onClick={() => handleRestore()}
                        disabled={changedText === original}
                        style={{ width: 200 }}
                    >
                        {spinning ? (
                            <Spinner size="sm" />
                        ) : (
                            <i className="bi bi-cloud-arrow-up" />
                        )}
                        <span className="ms-2">Save Changes</span>
                    </Button>
                </div>
            </div>
        </div>
    );
}
jaywcjlove commented 1 year ago

@kimfucious https://codesandbox.io/embed/https-github-com-uiwjs-react-codemirror-issues-516-0pxrih?fontsize=14&hidenavigation=1&theme=dark

You can use codesandbox.io to reproduce your error, I will help you to see.

kimfucious commented 1 year ago

Let me try... this might take a while.

kimfucious commented 1 year ago

Hi @jaywcjlove,

I can't seem to reproduce it in CodeSandbox, but stripped out a bunch of stuff like Redux state and some memoized values.

Maybe I'll try to add that back in to see if that's causing it.

I'm rendering the component conditionally, and it is removed from the DOM when the component unmounts.

So far it's a mystery.

kimfucious commented 1 year ago

Hi, @jaywcjlove.

I've discovered a couple of things:

React, running in dev with Strict Mode enabled, seems to "allow" the duplication, but it's not the root cause.

What I've figured out, by process of elimination, is that this line (from my initial post) is causing it to occur:

const { app } = useAppSelector((state) => state);

useAppSelector is a hook for Redux.

With that line enabled AND running the app w/o React Strict Mode, the problem only occurs if I navigate away from page and return (using React Router navigate). This adds a second CodeMirrorMerge element.

If I disable that line AND run the app w/o React Strict Mode, the problem goes away, permanently. The thing is that useAppSelector is needed by this component as "original" data is coming from Redux state.

Here's a screen capture of the behavior with the useAppSelector enabled and Strict Mode off.

https://github.com/uiwjs/react-codemirror/assets/26792769/2319ce1b-ef44-4274-b865-3e8c7bbda927

The large space upon return to the merge editor component is caused by the second element being added and the fact that I have set the .cm-mergeView CSS class with a height of 500px.

I cannot reproduce this in CodeSandbox yet, so I'm assuming there is still something in my code causing the issue.

I don't expect you to try and figure this out, but I'm posting my findings here for future record.