Update: Some UI changes for video player (#128)

1. Tabbing through settings menu no longer shows border over menu items (highlights instead). This is because borders/box-shadow/outline does not play well with table-rows on Safari and IE/Edge. Instead, we'll change background color. 2. CC icon now changes background color to indicate that subtitles are on, instead of the previous white underline. 3. Using overflow-y: auto so that scrollbar doesn't show on hover of settings menu in IE/Edge 4. Using div instead of span for cc-text so I can size it, because using the span resulted in too short of a CC icon on IE/Edge.
box · May 19, 2017 · 81802c1 · 81802c1
1 parent d45bb83
commit 81802c1
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 28 deletions.
diff --git a/src/lib/viewers/media/MediaControls.html b/src/lib/viewers/media/MediaControls.html
@@ -36,8 +36,7 @@
         <span class="bp-media-controls-label">&nbsp;&#47;&nbsp;</span>
         <label class="bp-media-controls-label bp-media-controls-duration">00:00</label>
         <button class="bp-media-controls-btn bp-media-cc-icon">
-            <span class="bp-media-controls-cc-icon-text">CC</span>
-            <div class="bp-media-cc-icon-on"></div>
+            <div class="bp-media-controls-cc-icon-text">CC</div>
         </button>
         <button class="bp-media-controls-btn bp-media-gear-icon" aria-haspopup="true">
             <svg width="24" height="24" viewBox="0 0 24 24" focusable="false">

diff --git a/src/lib/viewers/media/MediaControls.scss b/src/lib/viewers/media/MediaControls.scss
@@ -158,39 +158,34 @@
     }
 }
 
-.bp-media-cc-icon-on {
-    background-color: $white;
-    bottom: 6px;
-    display: inline-block;
-    height: 3px;
-    left: 12px;
-    opacity: 0;
-    position: absolute;
-    transition: opacity .3s;
-    width: 21px;
-}
-
 .bp-media-controls-cc-icon-text {
-    background: $white;
-    border-radius: 3px;
+    background-color: rgba($black, .3); // Anything less than .3 looks too transparent on IE/Edge
+    border-radius: 4px;
     bottom: 2px;
-    color: $black;
-    font-size: 11px;
+    color: $white;
+    display: inline-block;
+    font-size: 12px;
     font-weight: 600;
+    height: 22px;
     letter-spacing: .1em;
-    line-height: 8px;
-    padding: 2px 2px 2px 3px;
-    position: relative;
-}
+    line-height: 22px;
+    padding: 0 0 0 1px; // Padding left 1px because text isn't perfectly centered on Safari/Firefox even with text-align:center. Looks fine with extra-padding on Chrome/IE/Edge
+    position: relative; // For lining up the bottom with other buttons in the controls bar
+    width: 24px;
 
-.bp-media-settings-subtitles-on {
-    .bp-media-cc-icon-on {
-        opacity: 1;
+    &:hover {
+        background-color: $box-blue;
+        color: $white;
+    }
+
+    .bp-media-settings-subtitles-on & {
+        background-color: $box-blue;
+        color: $white;
     }
 }
 
 .bp-media-controls-hd {
-    background: $box-blue;
+    background-color: $box-blue;
     border-radius: 3px;
     display: none;
     font-size: 8px;

diff --git a/src/lib/viewers/media/Settings.scss b/src/lib/viewers/media/Settings.scss
@@ -15,7 +15,7 @@ $item-hover-color: #f6fafd;
     max-width: 400px;
     -ms-overflow-style: -ms-autohiding-scrollbar;
     overflow-x: hidden;
-    overflow-y: scroll;
+    overflow-y: auto; // So scrollbar doesn't show on hover in IE/Edge
     padding: $padding;
     position: absolute;
     right: 16px;
@@ -83,7 +83,16 @@ $item-hover-color: #f6fafd;
     }
 
     .bp-has-keyboard-focus &:focus {
-        box-shadow: inset 0 0 0 1px fade-out($black, .75); // Need a blacker border to contrast against white background
+        background-color: $box-blue;
+
+        .bp-media-settings-label,
+        .bp-media-settings-value {
+            color: $white;
+        }
+
+        svg {
+            fill: $white;
+        }
     }
 }