Skip to content

feat: empty equal cell style refactor #18

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

Merged
merged 1 commit into from
Sep 5, 2025
Merged

feat: empty equal cell style refactor #18

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
29 changes: 29 additions & 0 deletions src/components/DiffViewer/components/VirtualizedDiffViewer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export const VirtualizedDiffViewer: React.FC<VirtualizedDiffViewerProps> = ({
miniMapWidth,
inlineDiffOptions,
}) => {
const outerRef = useRef<Node>(null);
const listRef = useRef<List>(null);
const [scrollTop, setScrollTop] = useState(0);

Expand Down Expand Up @@ -167,6 +168,33 @@ export const VirtualizedDiffViewer: React.FC<VirtualizedDiffViewerProps> = ({
onScroll: (scrollTop: number) => listRef.current?.scrollTo(scrollTop),
};

// Observe DOM changes in the diff viewer and update cells with the "empty-equal-cell" class
// whenever new rows are rendered, ensuring virtualized/scroll-loaded cells are handled.
useEffect(() => {
const container = outerRef.current as HTMLElement | null;
if (!container)
return;

const observer = new MutationObserver(() => {
const tds = container?.querySelectorAll<HTMLTableCellElement>(
".json-diff-viewer td.line-equal",
);

tds.forEach((td, index) => {
td.classList.remove("empty-equal-cell");

const spans = td.querySelectorAll("pre > span");
if (index % 2 === 1 && spans.length === 1 && spans[0].textContent?.trim() === "") {
td.classList.add("empty-equal-cell");
}
});
});

observer.observe(container!, { childList: true, subtree: true });

return () => observer.disconnect();
}, []);
Comment on lines +173 to +196
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Sep 5, 2025

Choose a reason for hiding this comment

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

high

The current MutationObserver implementation can cause performance issues on large diffs and relies on fragile selectors.

  • Performance: On every DOM change (e.g., scrolling in the virtualized list), the observer callback re-queries the entire component for td.line-equal elements and iterates over all of them. This is inefficient and can lead to UI lag. A better approach is to process only the newly added nodes provided by the MutationObserver.
  • Fragile Selector: The logic to identify the right-side cell using index % 2 === 1 is implicit. It assumes a flat list of <td> elements in a specific order, which can easily break. It's more robust to use a specific CSS selector like :nth-child(4) to target the right-side content cell within its row.
  • Type Safety: Casting outerRef.current with as HTMLElement is less safe than using an instanceof type guard.

I suggest refactoring this useEffect to be more performant and robust by processing only added nodes and using more specific selectors.

  useEffect(() => {
    const container = outerRef.current;
    if (!(container instanceof HTMLElement)) {
      return;
    }

    const observer = new MutationObserver((mutations) => {
      for (const mutation of mutations) {
        if (mutation.type !== "childList") {
          continue;
        }

        for (const node of mutation.addedNodes) {
          if (!(node instanceof HTMLElement)) {
            continue;
          }

          const rightCell = node.querySelector<HTMLTableCellElement>(
            ".json-diff-viewer td.line-equal:nth-child(4)",
          );

          if (rightCell) {
            const spans = rightCell.querySelectorAll("pre > span");
            if (spans.length === 1 && spans[0].textContent?.trim() === "") {
              rightCell.classList.add("empty-equal-cell");
            }
          }
        }
      }
    });

    observer.observe(container, { childList: true, subtree: true });

    return () => observer.disconnect();
  }, []);

Copy link
Owner Author

@utkuakyuz utkuakyuz Sep 5, 2025

Choose a reason for hiding this comment

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

I cannot apply this because this creates a new display bug when expanding hidden lines. When user clicks on expand button, new displayed rows shown with background pattern. This is not wanted. Optimization may be considered in the future if any idea comes up.


return (
<div className={`diff-viewer-container${className ? ` ${className}` : ""}`}>
{/* Header & Search */}
Expand Down Expand Up @@ -208,6 +236,7 @@ export const VirtualizedDiffViewer: React.FC<VirtualizedDiffViewerProps> = ({
{/* List & Minimap */}
<div style={{ display: "flex", gap: "8px", position: "relative" }}>
<List
outerRef={outerRef}
ref={listRef}
className="virtual-json-diff-list-container"
height={height}
Expand Down
7 changes: 7 additions & 0 deletions src/components/DiffViewer/styles/JsonDiffCustomTheme.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
background: #282a36;
color: #f8f8f2;
width: 100%;
border-spacing: 0;
font-family:
-apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, "Helvetica Neue", Arial, "Noto Sans", sans-serif,
"Apple Color Emoji", "Segoe UI Emoji", "Segoe UI Symbol", "Noto Color Emoji";
Expand Down Expand Up @@ -95,6 +96,12 @@
background: rgba(182, 180, 67, 0.08);
}

.json-diff-viewer.json-diff-viewer-theme-custom tr .empty-equal-cell {
opacity: 0.4;
background: repeating-linear-gradient(-53deg, rgb(69, 69, 70), rgb(69, 69, 70) 1.5px, #282a36 1.5px, #282a36 4px);
background-attachment: fixed;
}

.json-diff-viewer.json-diff-viewer-theme-custom tr .line-number {
background: unset;
padding: 0 4px;
Expand Down