-
-
Notifications
You must be signed in to change notification settings - Fork 283
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
PR: Add transcripts to videos #181
Conversation
Congratulations @juanis2112! This looks awesome! I only have one small comment: I think there's not enough padding between the text and the scrollbar on the right My suggestions to avoid this are:
.scroll.docutils > p.card-text {
padding-right: 15px;
} With those two changes things look like this What do you think? |
Another option would be to remove the margins inside the div.scroll {
height: 250px;
overflow-x: hidden;
overflow-y: auto;
/* text-align: justify; */
font-size: 0.8em;
color: #2c2e30;
margin: -20px;
padding-top: 20px;
padding-left: 25px;
padding-right: 25px;
padding-bottom: 0px;
} I think I prefer this solution because it makes the scrollbar perfectly aligned with the border of the dropdown. |
4d45874
to
466c629
Compare
Note: This needs a rebase |
466c629
to
23a4f28
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great @juanis2112 ! Just a bunch of minor typos and formatting changes, almost all of them my own fault, sorry. The content and display of the PR output all looks good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM on my end, thanks @juanis2112 !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fantastic job @juanis2112 and @CAM-Gerlach! This looks really great!
I only have one minor comment. Most of the script of First Steps with Spyder > Getting started
is not the inside the div that should contain it:
Other than that, this is ready to me.
@ccordoba12 Fixed, sorry, it was an indentation mistake introduced by one of the minor changes I applied to the transcript text. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me now, thanks!
Pull Request Checklist
Description of Changes
In this PR I will add all the transcripts to the videos as an scrollable element under a dropdown.
The first 2 commits have the directives necessary to do this along with the first script but once @CAM-Gerlach is done reviewing and formatting all of them I will add the rest.
Let me know what you think about the design. I think I'll try to talk to Isabella for some advice on colors and format.
Issue(s) Resolved
Fixes #183