Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
- Switched CircleCI ImageMagick download to use http
- Modified CI config to take advantage of partial dependency caching and exploit parallelism when resolving/updating dependencies
- Migrate graph-related components (FocusBar, FocusTab, GraphDropdown, GraphFallback, Infobox, and Sidebar) from classes to functions
- Refactor GraphDropdown component from being a child of Graph to being a child of NavBar

## [0.7.1] - 2025-06-16

Expand Down
46 changes: 45 additions & 1 deletion js/components/common/NavBar.js.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,49 @@ import React from "react"
import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"
import { faDownload } from "@fortawesome/free-solid-svg-icons"
import { Tooltip } from "react-tooltip"
import GraphDropdown from "../graph/GraphDropdown"

/**
* NavBar component.
*/
export function NavBar({ selected_page, open_modal }) {
export function NavBar({ selected_page, open_modal, graphs = [], updateGraph }) {
const isActive = page => (page === selected_page ? "selected-page" : undefined)
const [showGraphDropdown, setShowGraphDropdown] = React.useState(false)
const [dropdownTimeouts, setDropdownTimeouts] = React.useState([])

const clearDropdownTimeouts = () => {
dropdownTimeouts.forEach(timeout => clearTimeout(timeout))
setDropdownTimeouts([])
}

const handleShowGraphDropdown = () => {
clearDropdownTimeouts()
setShowGraphDropdown(true)
}

const handleHideGraphDropdown = () => {
const timeout = setTimeout(() => {
setShowGraphDropdown(false)
}, 500)
setDropdownTimeouts(dropdownTimeouts.concat(timeout))
}

React.useEffect(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

The indentation at this line is incorrect

const navGraph = document.querySelector("#nav-graph")
Copy link
Contributor

Choose a reason for hiding this comment

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

Part of your task is to remove DOM manipulation. This includes accessing DOM elements direction and manually adding callbacks (addEventListener, below). Instead, these callbacks should be managed by the React components themselves. Make sure you understand the existing behaviour (on master) so that you can replicate it with your changes.


if (navGraph) {
navGraph.addEventListener("mouseenter", handleShowGraphDropdown)
navGraph.addEventListener("mouseleave", handleHideGraphDropdown)
}

return () => {
clearDropdownTimeouts()
if (navGraph) {
navGraph.removeEventListener("mouseenter", handleShowGraphDropdown)
navGraph.removeEventListener("mouseleave", handleHideGraphDropdown)
}
}
}, [])

return (
<nav className="row header">
Expand All @@ -28,6 +65,13 @@ export function NavBar({ selected_page, open_modal }) {
<ul id="nav-links">
<li id="nav-graph" className={isActive("graph")}>
<a href="/graph">Graph</a>
<GraphDropdown
showGraphDropdown={showGraphDropdown}
onMouseMove={handleShowGraphDropdown}
onMouseLeave={handleHideGraphDropdown}
graphs={graphs}
updateGraph={updateGraph}
/>
</li>
<li id="nav-grid" className={isActive("grid")}>
<a href="/grid">Grid</a>
Expand Down
2 changes: 1 addition & 1 deletion js/components/graph/Container.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ export default class Container extends React.Component {
render() {
return (
<div>
<NavBar selected_page="graph" open_modal={this.openExportModal}></NavBar>
<NavBar selected_page="graph" open_modal={this.openExportModal} graphs={this.state.graphs} updateGraph={this.updateGraph}></NavBar>
<ExportModal
page="graph"
open={this.state.modalOpen}
Expand Down
51 changes: 2 additions & 49 deletions js/components/graph/Graph.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,12 @@ import React from "react"
import PropTypes from "prop-types"
import { CourseModal } from "../common/react_modal.js.jsx"
import { ExportModal } from "../common/export.js.jsx"
import { getProgram } from "../common/utils.js"
import { getPost } from "../common/utils.js"
Copy link
Contributor

Choose a reason for hiding this comment

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

This change is unrelated to this PR

import Bool from "./Bool"
import Edge from "./Edge"
import Node from "./Node"
import Button from "./Button"
import InfoBox from "./InfoBox"
import GraphDropdown from "./GraphDropdown"
import Sidebar from "./Sidebar"
import { parseAnd } from "../../util/util.js"
import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"
Expand Down Expand Up @@ -44,7 +43,6 @@ export class Graph extends React.Component {
highlightedNodesFocus: [],
highlightedNodesDeps: [],
infoboxTimeouts: [],
dropdownTimeouts: [],
width: window.innerWidth,
height: window.innerHeight,
zoomFactor: 1,
Expand All @@ -68,7 +66,6 @@ export class Graph extends React.Component {
panStartX: 0,
panStartY: 0,
showCourseModal: false,
showGraphDropdown: false,
selectedNodes: new Set(),
}
this.exportModal = React.createRef()
Expand All @@ -83,15 +80,6 @@ export class Graph extends React.Component {
// can't detect keydown event when adding event listener to react-graph
document.body.addEventListener("keydown", this.onKeyDown)

if (document.querySelector("#nav-graph > a")) {
document
.querySelector("#nav-graph > a")
.addEventListener("mouseenter", this.setShowGraphDropdown)
document
.querySelector("#nav-graph > a")
.addEventListener("mouseleave", this.hideGraphDropdown)
}

if (document.querySelector(".sidebar")) {
document
.querySelector(".sidebar")
Expand All @@ -107,23 +95,13 @@ export class Graph extends React.Component {

componentWillUnmount() {
this.state.infoboxTimeouts.forEach(timeout => clearTimeout(timeout))
this.state.dropdownTimeouts.forEach(timeout => clearTimeout(timeout))
document.body.removeEventListener("keydown", this.onKeyDown)

if (document.getElementById("nav-export")) {
document
.getElementById("nav-export")
.removeEventListener("click", this.exportModal.current.openModal)
}

if (document.querySelector("#nav-graph > a")) {
document
.querySelector("#nav-graph > a")
.removeEventListener("mouseenter", this.setShowGraphDropdown)
document
.querySelector("#nav-graph > a")
.removeEventListener("mouseleave", this.hideGraphDropdown)
}
}

getGraph = () => {
Expand Down Expand Up @@ -341,7 +319,7 @@ export class Graph extends React.Component {
this.highlightFocuses([])
} else if (this.props.currFocus !== prevProps.currFocus) {
const currFocusCourses = this.state.focusCourses[this.props.currFocus]
getProgram(
getPost(
this.props.currFocus,
currFocusCourses?.modifiedTime || new Date(0).toUTCString()
).then(focusData => {
Expand All @@ -367,10 +345,6 @@ export class Graph extends React.Component {
this.state.infoboxTimeouts.forEach(timeout => clearTimeout(timeout))
this.setState({ infoboxTimeouts: [] })
break
case TIMEOUT_NAMES_ENUM.DROPDOWN:
Copy link
Contributor

Choose a reason for hiding this comment

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

It's now possible that TIMEOUT_NAMES_ENUM.DROPDOWN is no longer used anywhere. Check this, and if so please delete the DROPDOWN value from the enum.

this.state.dropdownTimeouts.forEach(timeout => clearTimeout(timeout))
this.setState({ dropdownTimeouts: [] })
break
}
}

Expand Down Expand Up @@ -693,20 +667,6 @@ export class Graph extends React.Component {
})
}

setShowGraphDropdown = () => {
this.clearAllTimeouts(TIMEOUT_NAMES_ENUM.DROPDOWN)
this.setState({ showGraphDropdown: true })
}

hideGraphDropdown = () => {
const timeout = setTimeout(() => {
this.setState({ showGraphDropdown: false })
}, 500)
this.setState({
dropdownTimeouts: this.state.dropdownTimeouts.concat(timeout),
})
}

onClose = () => {
this.setState({ showCourseModal: false })
}
Expand Down Expand Up @@ -1645,13 +1605,6 @@ export class Graph extends React.Component {
onClose={this.onClose}
/>
<ExportModal context="graph" session="" ref={this.exportModal} />
<GraphDropdown
showGraphDropdown={this.state.showGraphDropdown}
onMouseMove={this.setShowGraphDropdown}
onMouseLeave={this.hideGraphDropdown}
graphs={this.props.graphs}
updateGraph={this.props.updateGraph}
/>
{Object.keys(this.state.nodesJSON).length > 1 && (
<div className="graph-button-group">
<div className="button-group">
Expand Down
3 changes: 2 additions & 1 deletion js/components/graph/GraphDropdown.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ export default function GraphDropdown({showGraphDropdown, onMouseMove, onMouseLe
onMouseMove={onMouseMove}
onMouseLeave={onMouseLeave}
data-testid={"test-graph-dropdown"}
style={{ left: graphTabLeft }}
style={{ left: graphTabLeft, top: "50px", zIndex: 1000
}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Put the }} on the same line as the {{.

Also, in this file there is still direct DOM manipulation, which you should remove by handling the logic in the parent NavBar component.

>
{graphs.map((graph, i) => {
return (
Expand Down