Skip to content

Conversation

agbilotia1998
Copy link
Member

@agbilotia1998 agbilotia1998 commented May 23, 2018

Checklist

Short description of what this resolves:

Adds option to view slides of the session

Fixes #1809

@ghost ghost added the needs-review label May 23, 2018
@ghost ghost assigned agbilotia1998 May 23, 2018
@agbilotia1998 agbilotia1998 requested a review from dilpreetsio May 23, 2018 07:29
position: absolute;
right: 23px;
top: 1px;
.video, .slides {

Choose a reason for hiding this comment

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

Each selector in a comma sequence should be on its own single line

@@ -225,11 +225,17 @@ a {
}

@media (min-width: 600px) {
.video-iframe {
.video-iframe , .iframe {

Choose a reason for hiding this comment

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

Each selector in a comma sequence should be on its own single line

@codecov
Copy link

codecov bot commented May 23, 2018

Codecov Report

Merging #1878 into development will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##           development    #1878   +/-   ##
============================================
  Coverage        72.81%   72.81%           
============================================
  Files               10       10           
  Lines             2358     2358           
  Branches           416      416           
============================================
  Hits              1717     1717           
  Misses             641      641
Impacted Files Coverage Δ
src/backend/fold_v2.js 92.6% <ø> (ø) ⬆️

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 8dbcff3...21d7d2a. Read the comment docs.

@mariobehling
Copy link
Member

Let's follow best practices more closely as we progress. Please provide screenshots and test links.

@agbilotia1998
Copy link
Member Author

@mariobehling though slides URL are not available for any event, I tested it using dummy data. I am posting here a screenshot of the slideshow with test data -
screenshot-2018-5-23 mozilla all hands 2017

@mariobehling
Copy link
Member

Asking again also in this issue.

Let's follow best practices more closely as we progress. Please provide screenshots and test links.

@agbilotia1998
Copy link
Member Author

@dilpreetsio
Copy link
Member

@agbilotia1998 Can we try to use an open source viewer for the slides?

@agbilotia1998
Copy link
Member Author

@geekyd I searched for many slide-viewers but there are open source alternatives for pdf slides only, LibreOffice also does not any such iframe or embedded object in HTML. Any suggestions from your side?

Copy link
Member

@dilpreetsio dilpreetsio left a comment

Choose a reason for hiding this comment

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

Merging it for now. We will iterate from here.

@ghost ghost removed the needs-review label May 27, 2018
@dilpreetsio dilpreetsio merged commit 551f75c into fossasia:development May 27, 2018
@ghost ghost added the ready-to-ship label May 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add "Slides" button to session and adapt design accordingly
4 participants