NUKnightLab / NUKnightLab/TimelineJS3
Deprecate / remove DOMUtil
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
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- 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