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

Adds feature to view slides of the session #1878

Merged
merged 5 commits into from May 27, 2018

Conversation

4 participants
@agbilotia1998
Copy link
Member

commented May 23, 2018

Checklist

Short description of what this resolves:

Adds option to view slides of the session

Fixes #1809

position: absolute;
right: 23px;
top: 1px;
.video, .slides {

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot May 23, 2018

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 {

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot May 23, 2018

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

@agbilotia1998 agbilotia1998 force-pushed the agbilotia1998:slidesPPT branch from cba186f to ea09969 May 23, 2018

@codecov

This comment has been minimized.

Copy link

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

This comment has been minimized.

Copy link
Member

commented May 23, 2018

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

@agbilotia1998

This comment has been minimized.

Copy link
Member Author

commented May 23, 2018

@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

This comment has been minimized.

Copy link
Member

commented May 25, 2018

Asking again also in this issue.

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

@agbilotia1998

This comment has been minimized.

Copy link
Member Author

commented May 25, 2018

@geekyd

This comment has been minimized.

Copy link
Member

commented May 25, 2018

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

@agbilotia1998

This comment has been minimized.

Copy link
Member Author

commented May 25, 2018

@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?

agbilotia1998 added some commits May 26, 2018

@geekyd

geekyd approved these changes May 27, 2018

Copy link
Member

left a comment

Merging it for now. We will iterate from here.

@open-event-bot open-event-bot bot removed the needs-review label May 27, 2018

@geekyd geekyd merged commit 551f75c into fossasia:development May 27, 2018

5 checks passed

Codacy/PR Quality Review Up to standards. A positive pull request.
Details
Hound No violations found. Woof!
codecov/patch Coverage not affected when comparing 8dbcff3...21d7d2a
Details
codecov/project 72.81% remains the same compared to 8dbcff3
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.