Skip to content
This repository was archived by the owner on Jan 15, 2020. It is now read-only.

Add Presentation Section to Resources Page #63

Merged
merged 2 commits into from
Mar 19, 2019
Merged

Conversation

nabdelgadir
Copy link
Contributor

@nabdelgadir nabdelgadir commented Mar 11, 2019

Closes #61.

What the new section looks like:

Screen Shot 2019-03-11 at 1 22 50 PM

  • The slides are embed so anyone can look at them in the page.
    • I put the one set we have on the left assuming we'll add more in the future, but I can centre it if that's better.
  • The videos couldn't be embed because Node Summit didn't allow it, so I had to just link to them instead.
  • Any better suggestions for the title/description?

An alternate implementation to get rid of the white space until we have more content:

Screen Shot 2019-03-11 at 6 39 05 PM

@nabdelgadir nabdelgadir requested a review from dhmlau March 11, 2019 22:40
@nabdelgadir nabdelgadir self-assigned this Mar 11, 2019
Signed-off-by: Nora <nora.abdelgadir@ibm.com>
@dhmlau
Copy link
Member

dhmlau commented Mar 12, 2019

@nabdelgadir, thanks for taking up this task!
I haven't got a chance to go through the PR in details, but I have a few thoughts:

  • it might be better to go by the event date, rather to categorize them into videos and slides. There might be talks that have both. :)
  • NodeSummit presentation videos are shared through vimeo. Not sure if it help: https://help.vimeo.com/hc/en-us/articles/224969968-Embedding-videos-overview
  • I'm thinking along the line that each tile contains all the information (event name, date, speaker, slides URL, video URL)

@b-admike
Copy link
Member

@nabdelgadir, thanks for taking up this task!
I haven't got a chance to go through the PR in details, but I have a few thoughts:

  • it might be better to go by the event date, rather to categorize them into videos and slides. There might be talks that have both. :)
  • NodeSummit presentation videos are shared through vimeo. Not sure if it help: https://help.vimeo.com/hc/en-us/articles/224969968-Embedding-videos-overview
  • I'm thinking along the line that each tile contains all the information (event name, date, speaker, slides URL, video URL)

Agreed 👍. For the last point, however, won't that be too much info? Also, I like the version of this patch where there is no whitespace for now.

@nabdelgadir
Copy link
Contributor Author

@dhmlau I haven't pushed this code, but what do you think about this (pending changing the icon of Raymond's slides to something else)? Currently if you press on any of the cards it links you to the page with the content (e.g. http://www.nodesummit.com/prior-video/node-summit-2018-async-functions-in-practice-miroslav-bajtos/).
Screen Shot 2019-03-12 at 7 45 53 PM

As for the videos, unfortunately I keep getting this 😢:
Screen Shot 2019-03-12 at 7 25 07 PM

From vimeo: "The owner of this video has set domain restrictions and you will not be able to embed it on all websites. Please contact the owner if you would like permission to embed this video."

@dhmlau
Copy link
Member

dhmlau commented Mar 15, 2019

@nabdelgadir, almost there!

  • since we won't be able to embed the video for the NodeSumit ones, I'd prefer not to have others embedded, so that we have a consistent look of the tiles and it's not taking too much space for particular tiles in this section.
  • let's call the meetup as Toronto Meetup 2019
  • For Janny's video+slide, can we make it in the same tile as we discussed before? i.e. there's a line below the Janny Hou | Toronto Meetup 2019:
recording | slides

resources.html Outdated
<div class="col-md-6">
<p style="height:6px;"></p>
<div class="text-center" style="justify-content: center">
<div class="card-2 pres multiple-links" onclick="location.href='https://www.youtube.com/watch?v=i3UW5PhDoQg';">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This part is kinda hacky because it won't let you put more than one link in a card or else it'll break up the card.

@nabdelgadir
Copy link
Contributor Author

@dhmlau thoughts?

Screen Shot 2019-03-19 at 8 32 04 AM

@dhmlau
Copy link
Member

dhmlau commented Mar 19, 2019

Looks good. It would be even better if we could make the slides row height a bit smaller (or padding smaller), so that it's more compact. Thanks.

Copy link
Member

@dhmlau dhmlau left a comment

Choose a reason for hiding this comment

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

One minor comment - I'd prefer the slides link (from Janny's tile) will open a new tab, just like clicking the other tile. Other than that, LGTM.
Please squash the commits before merging. Thanks!

Signed-off-by: Nora <nora.abdelgadir@ibm.com>
@nabdelgadir nabdelgadir merged commit 92ac1de into master Mar 19, 2019
@nabdelgadir nabdelgadir deleted the presentations branch March 19, 2019 17:09
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants