NUKnightLab / NUKnightLab/TimelineJS3

Deprecate / remove DOMUtil

Open
#799 9 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
JavaScript
Stars
3.2k
Forks
645
PR merge metrics
No merged PRs in 30d

Description

The DomUtil class is unnecessary. It has three functions which are better handled using standard DOM Methods:

Calls to DOMUtil.addClass(el, name) could be replaced with el.classList.add(name)

Calls to DOMUtil.removeClass(el,name) could be replaced with el.classList.remove(name)

Calls to DOMUtil.hasClass(el,name) could be replaced with el.classList.contains(name)

Before today, that left only isInHorizontalViewport(element), which is only used in TimeAxis... so instead of leaving it hanging around, I just moved it.

If I felt like we had a better method of testing UX changes, I might have gone ahead and put in those changes above, at least within DOMUtil but it feels a bit risky.

This issue's title references deprecation, but I don't even know if that's necessary. Does anyone really use anything that deep in the "public" API for TimelineJS?

(This issue inspired by some code inspection around integrating #798)

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with src/js/dom/DOMUtil.js and inspect its callers, especially TimeAxis, to determine whether the utility is still used or exposed as part of TimelineJS's public API. Trace the class helpers and the moved isInHorizontalViewport usage, then confirm that callers can use standard DOM methods and that the utility can be removed or deprecated without breaking the project.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
frontend
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.