Refactor Browser God Object #404

turner · 2025-11-12T16:14:18Z

No description provided.

…sibilities

…n Cursor

…support

Copilot

Pull Request Overview

This PR refactors the HICBrowser "God Object" by extracting distinct responsibilities into dedicated coordinator/manager classes following the Single Responsibility Principle. The refactoring introduces StateManager, NotificationCoordinator, InteractionHandler, DataLoader, and RenderCoordinator to separate state management, UI notifications, user interactions, data loading, and rendering concerns.

Key Changes

Extracted state management into StateManager class for centralized dataset and state handling
Created NotificationCoordinator to handle all UI component notifications
Introduced InteractionHandler for user interaction methods (goto, zoom, pan)
Added DataLoader to manage Hi-C file, track, and normalization data loading
Implemented RenderCoordinator to handle rendering coordination and queuing

Reviewed Changes

Copilot reviewed 36 out of 36 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
js/hicBrowser.js	Refactored to delegate responsibilities to new coordinator classes while maintaining backward compatibility
js/stateManager.js	New class managing active dataset, state transitions, and cross-browser synchronization
js/notificationCoordinator.js	New class coordinating UI component updates when browser state changes
js/interactionHandler.js	New class handling navigation, zoom, and pan operations
js/dataLoader.js	New class managing data loading operations for Hi-C files, tracks, and normalizations
js/renderCoordinator.js	New class coordinating rendering with queuing logic for rapid calls
js/hicState.js	Added helper methods for state validation, pixel size adjustment, and update finalization
js/urlUtils.js	Added session loading from File objects and file paths with URL shortcut expansion
js/session.js	Added URL shortcut expansion for backward compatibility
test/testSession.js	New test file for session loading functionality
vite.config.js	Added plugins to suppress source map warnings for third-party dependencies
various widget files	Added helper methods to encapsulate DOM manipulation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-12T16:19:04Z

dev/dat-sequence-gene-track.html

        }

-    juicebox.init(document.getElementById("app-container"), config_deep_map)
+    juicebox.init(document.getElementById("app-container"), config)


Variable config_deep_map is changed to config, but config is not defined in this file. This will cause a ReferenceError at runtime. Either define config or revert to config_deep_map.

@copilot open a new pull request to apply changes based on this feedback

Copilot · 2025-11-12T16:19:05Z

js/dataLoader.js

+        } catch (error) {
+            this.browser.contactMapLabel.textContent = "";
+            this.browser.contactMapLabel.title = "";
+            config.name = name;


Variable name is used before it's defined. The line config.name = name references name from line 84, but this code is in the catch block after the try block where name was defined. If an error occurs before line 84, name will be undefined, causing this assignment to fail silently or set config.name to undefined.

Copilot · 2025-11-12T16:19:05Z

vite.config.js

+  customLogger: {
+    warn(msg, options) {
+      // Filter out source map warnings for igv dependencies
+      const message = typeof msg === 'string' ? msg : String(msg);
+      if (message.includes('igv-ui.css.map') || 
+          message.includes('Failed to load source map') ||
+          (message.includes('ENOENT') && message.includes('igv'))) {
+        return; // Suppress these warnings
+      }
+      // Use default warning logger for other messages
+      console.warn(msg, options);
+    },
+    error(msg, options) {
+      // Filter out source map errors for igv dependencies
+      const message = typeof msg === 'string' ? msg : String(msg);
+      if (message.includes('igv-ui.css.map') || 
+          message.includes('Failed to load source map') ||
+          (message.includes('ENOENT') && message.includes('igv'))) {
+        return; // Suppress these errors
+      }
+      // Use default error logger for other messages
+      console.error(msg, options);


The custom logger's warn and error methods call console.warn(msg, options) and console.error(msg, options). However, the standard console methods don't accept an options parameter in this way. This may cause issues or unexpected behavior. Consider spreading the arguments or checking if options is defined before passing it.

Copilot · 2025-11-12T16:19:05Z

dev/big_viewport.html

        "height": 1000,
        "url": "https://www.encodeproject.org/files/ENCFF799QGA/@@download/ENCFF799QGA.hic",
-        "name": "in-situ agar GM12878 MboI experiment",
+        // "name": "in-situ agar GM12878 MboI experiment",


Property "name" is commented out but should either be removed entirely or have an explanation for why it's commented out. Leaving commented-out code reduces maintainability.

Suggested change

// "name": "in-situ agar GM12878 MboI experiment",

Copilot · 2025-11-12T16:19:06Z

js/interactionHandler.js

+                for (i = 0; i < resolutions.length; i++) {
+                    if (this.browser.state.zoom === resolutions[i].index) break;
+                }
+                if (i !== undefined) {


This guard always evaluates to true.

Suggested change

if (i !== undefined) {

if (i < resolutions.length) {

Copilot · 2025-11-12T16:19:06Z

js/notificationCoordinator.js

+     * @param {boolean} eventData.dragging - Whether currently dragging
+     */
+    notifyLocusChange(eventData) {
+        const { state, resolutionChanged, chrChanged, dragging } = eventData;


Unused variable dragging.

Suggested change

const { state, resolutionChanged, chrChanged, dragging } = eventData;

const { state, resolutionChanged, chrChanged } = eventData;

Copilot · 2025-11-12T16:19:07Z

js/renderCoordinator.js

+            this.updating = false;
+            if (this.pending.size > 0) {
+                const queued = [];
+                for (let [k, v] of this.pending) {


Unused variable k.

Suggested change

for (let [k, v] of this.pending) {

for (let v of this.pending.values()) {

Copilot · 2025-11-12T16:19:07Z

test/testSession.js

+
+import { describe, test, expect } from 'vitest';
+import { extractConfig } from "../js/urlUtils.js";
+import { restoreSession } from "../js/session.js";


Unused import restoreSession.

Copilot · 2025-11-12T16:22:51Z

@turner I've opened a new pull request, #405, to work on those changes. Once the pull request is ready, I'll request review from you.

turner added 16 commits November 7, 2025 12:45

replace internal events with explicit methods. Clean up method respon…

b2d21a2

…sibilities

fowler refactors: phase 1

bd7b519

fowler refactors: phase 2

57193fd

fowler refactors: phase 3

e7d2206

fowler refactors: phase 4A and 4B

911836a

fowler refactors: phase 4C

0092abe

fowler refactors: phase 4D and 4E

c5f7ca2

gardening

c88184a

Added session loading test

cb16304

Add workspace file

caaeb7d

Added Workspace file to enable Juicebox and Spacewalk to be visible i…

97d9a63

…n Cursor

update workspace file

5a10832

un console log

0fab4d3

Refactor interactionHandler and HicState

d09bdf1

Add zoom wheel support. Nice.

c0a0f11

Add wheel zoom functionality with smooth zooming and synchronization …

20a73e2

…support

turner requested a review from Copilot November 12, 2025 16:14

Copilot started reviewing on behalf of turner November 12, 2025 16:15 View session

Copilot finished reviewing on behalf of turner November 12, 2025 16:17

Copilot AI reviewed Nov 12, 2025

View reviewed changes

Copilot AI mentioned this pull request Nov 12, 2025

[WIP] Address feedback on 'Refactor Browser God Object' PR #405

Closed

5 tasks

turner closed this Nov 12, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor Browser God Object #404

Refactor Browser God Object #404

Uh oh!

turner commented Nov 12, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Nov 12, 2025

Uh oh!

turner Nov 12, 2025

Uh oh!

Copilot AI Nov 12, 2025

Uh oh!

Copilot AI Nov 12, 2025

Uh oh!

Copilot AI Nov 12, 2025

Uh oh!

Copilot AI Nov 12, 2025

Uh oh!

Copilot AI Nov 12, 2025

Uh oh!

Copilot AI Nov 12, 2025

Uh oh!

Copilot AI Nov 12, 2025

Uh oh!

Copilot AI commented Nov 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	const { state, resolutionChanged, chrChanged, dragging } = eventData;
	const { state, resolutionChanged, chrChanged } = eventData;

	for (let [k, v] of this.pending) {
	for (let v of this.pending.values()) {

Refactor Browser God Object #404

Refactor Browser God Object #404

Uh oh!

Conversation

turner commented Nov 12, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Key Changes

Reviewed Changes

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

turner Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Nov 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants