Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: update the demo page #1184

Merged
merged 18 commits into from
Aug 17, 2021
Merged

chore: update the demo page #1184

merged 18 commits into from
Aug 17, 2021

Conversation

gkatsev
Copy link
Member

@gkatsev gkatsev commented Aug 16, 2021

Using bootstrap styles and tabs, update the demo page and bring in some stuff from the stats page.

The two main things that are missing right now are the bitrate switching and the timed metadata graphs, though, I think those aren't used as much. I figured we can bring what I have so far, and we can work on the others later on. For example, I want to migrate the representations dropdown to be a bit closer to what's in the stats page instead.

I kept the old index page as old-index.html and have the main page redirect there if we're on IE11, because the bootstrap version used doesn't support IE11. Hopefully, we won't be supporting IE11 for long, and we can delete this once we drop support.

@codecov
Copy link

codecov bot commented Aug 16, 2021

Codecov Report

Merging #1184 (15d6fd9) into main (a11f258) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1184   +/-   ##
=======================================
  Coverage   86.51%   86.51%           
=======================================
  Files          39       39           
  Lines        9602     9602           
  Branches     2219     2219           
=======================================
  Hits         8307     8307           
  Misses       1295     1295           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a11f258...15d6fd9. Read the comment docs.

@@ -77,7 +77,7 @@
var option = document.createElement('option');

option.innerText = 'HLS Manifest Object Test, does not survive page reload';
option.value = `data:application/vnd.videojs.vhs+json,${hlsManifest}`;
option.value = 'data:application/vnd.videojs.vhs+json,' + hlsManifest;
Copy link
Member Author

Choose a reason for hiding this comment

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

this broke IE11.

@@ -0,0 +1,707 @@
/* global window document */
Copy link
Member Author

Choose a reason for hiding this comment

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

this file is mostly a copy of old-index.js (previously index-demo-page.js). I'll point out new pieces below.

representationsEl.selectedIndex = selectedIndex;
};

function getBuffered(buffered) {
Copy link
Member Author

Choose a reason for hiding this comment

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

this helper method is from utils/stats and used for the Player Stats piece.

return bufferedText;
}

var setupSegmentMetadata = function(player) {
Copy link
Member Author

Choose a reason for hiding this comment

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

segment metadata from utils/stats

var li = document.createElement('li');
var pre = document.createElement('pre');

pre.classList.add('border', 'rounded', 'p-2');
Copy link
Member Author

Choose a reason for hiding this comment

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

this is a new piece for styling via bootstrap.

@@ -0,0 +1,150 @@
<!DOCTYPE html>
Copy link
Member Author

Choose a reason for hiding this comment

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

this used to be index.html

@@ -4,142 +4,228 @@
<meta charset="utf-8">
<title>videojs-http-streaming Demo</title>
<link rel="icon" href="logo.svg">
<link href="node_modules/bootstrap/dist/css/bootstrap.css" rel="stylesheet">
Copy link
Member Author

Choose a reason for hiding this comment

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

this file was a copy from index.html originally.

<link href="node_modules/video.js/dist/video-js.css" rel="stylesheet">
<link href="node_modules/videojs-http-source-selector/dist/videojs-http-source-selector.css" rel="stylesheet">
<style>
body {
Copy link
Member Author

Choose a reason for hiding this comment

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

a lot less custom styles are needed now.

</head>
<body class="m-4">
<script>
// if we're on IE, load up the load index page
Copy link
Member Author

Choose a reason for hiding this comment

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

redirect to the IE11 supporting demo page on IE.

<option>metadata</option>
</select>
</div>
<header class="container-fluid">
Copy link
Member Author

Choose a reason for hiding this comment

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

add a nice header with a link to the github

Copy link
Contributor

@brandonocasey brandonocasey left a comment

Choose a reason for hiding this comment

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

Perhaps the addition of the stats page stuff should be done in another pull request?

@gkatsev
Copy link
Member Author

gkatsev commented Aug 16, 2021

Perhaps the addition of the stats page stuff should be done in another pull request?

yup, my plan was to get it into a decent shape to merge and then different pieces can be done in separate PRs so that we're not holding anything back.

@gkatsev gkatsev merged commit 55f0bde into main Aug 17, 2021
@gkatsev gkatsev deleted the demo-page branch August 17, 2021 16:16
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.

2 participants