From f43577a2dd617934717fc35e222b22f44cfb9ccf Mon Sep 17 00:00:00 2001
From: infinite-persistence <inf.persistence@gmail.com>
Date: Tue, 7 Jun 2022 11:16:45 +0800
Subject: [PATCH] Variable naming and doc cleanup

- The function name is good and self-documenting, so removed the redundant comment. Whether it's 'precise' or not, it could change in the future and not worth maintaining a comment like that.
  - Focused on the non-obvious reason for SHIFT key instead.

- Fixed "on scroll" variable naming since it is not just for scrolling (it applies to keyboard up/down too)
---
 .../videoViewer/internal/videojs-shortcuts.jsx  | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/ui/component/viewers/videoViewer/internal/videojs-shortcuts.jsx b/ui/component/viewers/videoViewer/internal/videojs-shortcuts.jsx
index 3e613e118..326645558 100644
--- a/ui/component/viewers/videoViewer/internal/videojs-shortcuts.jsx
+++ b/ui/component/viewers/videoViewer/internal/videojs-shortcuts.jsx
@@ -6,8 +6,8 @@ import isUserTyping from 'util/detect-typing';
 
 const SEEK_STEP_5 = 5;
 const SEEK_STEP = 10; // time to seek in seconds
-const VOLUME_CHANGE_ON_SCROLL = 0.05;
-const PRECISE_VOLUME_CHANGE_ON_SCROLL = 0.01;
+const VOLUME_STEP = 0.05;
+const VOLUME_STEP_FINE = 0.01;
 
 // check if active (clicked) element is part of video div, used for keyboard shortcuts (volume etc)
 function activeElementIsPartOfVideoElement() {
@@ -16,7 +16,7 @@ function activeElementIsPartOfVideoElement() {
   return videoElementParent.contains(activeElement);
 }
 
-function volumeUp(event, playerRef, checkIsActive = true, amount = 0.05) {
+function volumeUp(event, playerRef, checkIsActive = true, amount = VOLUME_STEP) {
   // dont run if video element is not active element (otherwise runs when scrolling using keypad)
   const videoElementIsActive = activeElementIsPartOfVideoElement();
   const player = playerRef.current;
@@ -27,7 +27,7 @@ function volumeUp(event, playerRef, checkIsActive = true, amount = 0.05) {
   player.userActive(true);
 }
 
-function volumeDown(event, playerRef, checkIsActive = true, amount = 0.05) {
+function volumeDown(event, playerRef, checkIsActive = true, amount = VOLUME_STEP) {
   // dont run if video element is not active element (otherwise runs when scrolling using keypad)
   const videoElementIsActive = activeElementIsPartOfVideoElement();
   const player = playerRef.current;
@@ -166,10 +166,10 @@ const VideoJsShorcuts = ({
   }
 
   const handleVideoScrollWheel = (event, playerRef, containerRef) => {
-    // Handle precise volume control when scrolling over the video player while holding down the "SHIFT"-key
     const player = playerRef.current;
     const videoNode = containerRef.current && containerRef.current.querySelector('video');
 
+    // SHIFT key required. Scrolling the page will be the priority.
     if (!videoNode || !player || isUserTyping() || !event.shiftKey) return;
 
     event.preventDefault();
@@ -177,14 +177,13 @@ const VideoJsShorcuts = ({
     const delta = event.deltaY;
 
     if (delta > 0) {
-      volumeDown(event, playerRef, false, PRECISE_VOLUME_CHANGE_ON_SCROLL);
+      volumeDown(event, playerRef, false, VOLUME_STEP_FINE);
     } else if (delta < 0) {
-      volumeUp(event, playerRef, false, PRECISE_VOLUME_CHANGE_ON_SCROLL);
+      volumeUp(event, playerRef, false, VOLUME_STEP_FINE);
     }
   };
 
   const handleVolumeBarScrollWheel = (event, volumeElement, playerRef, containerRef) => {
-    // Handle generic and precise volume control when scrolling over the volume bar
     const player = playerRef.current;
     const videoNode = containerRef.current && containerRef.current.querySelector('video');
 
@@ -194,7 +193,7 @@ const VideoJsShorcuts = ({
     event.stopImmediatePropagation();
 
     const delta = event.deltaY;
-    const changeAmount = event.shiftKey ? PRECISE_VOLUME_CHANGE_ON_SCROLL : VOLUME_CHANGE_ON_SCROLL;
+    const changeAmount = event.shiftKey ? VOLUME_STEP_FINE : VOLUME_STEP;
 
     if (delta > 0) {
       volumeDown(event, playerRef, false, changeAmount);