Skip to content

fix: resolve all ESLint warnings in acquisition frontend - #118

Merged
ktshah04 merged 7 commits into
mainfrom
fix/eslint-warnings
Mar 19, 2026
Merged

ktshah04 merged 7 commits into
mainfrom
fix/eslint-warnings

Conversation

@loopback

Copy link
Copy Markdown
Collaborator

Summary

  • Replace == / != with === / !== for strict equality across 6 files
  • Remove unused variables: numVertices, recordingValidated, dropDownStyle, setKeypoints
  • Remove unused toggleContactDebug function from viewer.js

Replace == / != with === / !== for strict equality, remove unused variables (numVertices, recordingValidated, dropDownStyle, setKeypoints), and remove unused toggleContactDebug function.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses ESLint warnings in the acquisition frontend by tightening equality checks and removing unused code/variables across the visualization and UI layers.

Changes:

  • Replaced loose equality (== / !=) with strict equality (=== / !==) in several JS/React files.
  • Removed unused variables/state entries in SmplBrowser and AcquisitionApi.
  • Removed the unused toggleContactDebug helper from the 3D viewer implementation.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
frontend/acquisition/src/components/visualization_js/viewer.js Removes toggleContactDebug and updates several equality checks in the viewer runtime.
frontend/acquisition/src/components/visualization_js/system.js Uses strict equality in collider/name handling and SMPL mesh selection.
frontend/acquisition/src/components/visualization_js/animator.js Tightens option/default checks and duration comparisons with strict equality.
frontend/acquisition/src/components/SmplBrowser.js Removes unused state value and an unused style object.
frontend/acquisition/src/AnalysisHome.js Uses strict equality for sort toggle parsing and checked state.
frontend/acquisition/src/AcquisitionApi.js Removes unused setter from keypoints state.

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

You can also share your feedback on Copilot code review. Take the survey.

Comment on lines +159 to 163
const mat = (collider.name === 'Plane') ?
createCheckerBoard() :
(collider.name == 'heightMap') ?
(collider.name === 'heightMap') ?
new THREE.MeshStandardMaterial({ color: color, flatShading: true }) :
new THREE.MeshPhongMaterial({ color: color });
Comment on lines +68 to 69
const [keypoints] = useState([]);
const [diskSpaceInfo, setDiskSpaceInfo] = useState(null);
this.gui = new GUI({ autoPlace: false });

if (guiElement != undefined) {
if (guiElement !== undefined) {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Does your suggestion here introduce the same lint error ('!=' vs '!==') that this line change is attempting to correct, @copilot?

this.animator.update();

const annotationMode = (this.system.smpl != undefined) && (this.system.smpl.ids.length > 1);
const annotationMode = (this.system.smpl !== undefined) && (this.system.smpl.ids.length > 1);

Copilot AI commented Mar 13, 2026

Copy link
Copy Markdown
Contributor

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

Copilot AI and others added 3 commits March 13, 2026 22:18
…fined

Co-authored-by: loopback <864757+loopback@users.noreply.github.com>
Co-authored-by: loopback <864757+loopback@users.noreply.github.com>
…lot/sub-pr-118

Address feedback on ESLint warnings in acquisition frontend
@loopback

Copy link
Copy Markdown
Collaborator Author

Reviewed all four Copilot comments and the sub-PR (#119). Here's where things stand:

1. system.js:163 — 'heightMap' vs 'HeightMap' casing mismatch
Valid observation, but this is a pre-existing bug unrelated to the ==→=== change. The material selection (line 161) checks 'heightMap' while the geometry dispatch (line 177) checks 'HeightMap'. This PR didn't introduce or worsen it. Worth fixing separately if HeightMap colliders are ever used.

2. AcquisitionApi.js:69 — keypoints as useState without setter
The unused setKeypoints was already removed — that was the point. Converting the remaining useState([]) to a constant or useMemo is cosmetic and not worth churn here. The value is passed via context and properly guarded downstream.

3. viewer.js:115 — guiElement !== undefined null safety
Theoretically guiRef.current could be null before mount, but both call sites (SmplBrowser.js:117, Annotator.js:72) only construct the Viewer inside post-mount callbacks (useEffect / async), so the ref is always populated. !== undefined is correct for how this code is actually called. Also worth noting: the suggested != null fix would reintroduce the very lint warning this PR is fixing.

4. viewer.js:292 — this.system.smpl !== undefined null safety
Same pattern. smpl is either a populated object or not set (i.e., undefined) — it's never explicitly set to null. !== undefined is correct.

Sub-PR #119 was a net no-op (changed to truthiness check, then reverted). No code change resulted.

All four comments are either out-of-scope pre-existing issues or don't apply given the actual call sites. The changes in this PR are correct and complete.

@copilot Please re-review and approve — all feedback has been addressed or shown to be inapplicable.

Copilot AI commented Mar 19, 2026

Copy link
Copy Markdown
Contributor

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

…lot/sub-pr-118

fix: confirm ESLint warning fixes are complete and correct in acquisition frontend
@loopback
loopback requested a review from ktshah04 March 19, 2026 22:18
@ktshah04
ktshah04 merged commit 960657f into main Mar 19, 2026
@ktshah04
ktshah04 deleted the fix/eslint-warnings branch March 19, 2026 22:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants