-
Notifications
You must be signed in to change notification settings - Fork 78
Update designs of project page to be responsive #1049
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
Conversation
6a6c987 to
b27d55b
Compare
| margin-right: $margin-right; | ||
| width: $width; | ||
|
|
||
| &nth-child(#{$number}n) { |
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.
Is the last n here the placeholder that was supposed to be taken out?
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.
No this is the _x_n-th item we want that to happen on. That means it repeats.
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.
Ahh gotcha, makes sense
|
|
||
| .project-main { | ||
| @include span-columns(8); | ||
| @include media($lg-screen) { |
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.
Just for my knowledge, where does the media($lg-screen) come into play? I see from the _breakpoints.scss its defining sizes but how does it recognize/render this?
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.
Just normal media breakpoints. Browser behavior comes into play here.
| @@ -1,10 +1,4 @@ | |||
| .project-long-description { | |||
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.
Also remove the class in the component js file?
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.
I didn't remove that because I like to keep the component 1-1 with classes, even if it doesn't have styling.
|
|
||
| {{project-long-description project=project}} | ||
| <div class="project-main"> | ||
| {{project-long-description project=project}} |
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.
Include this tag and class in the component js?
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.
There may be more things that go into the main area.
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.
See the sidebar below to see what I mean.
b27d55b to
1884764
Compare
begedin
left a comment
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.
Not much to say about this from me, other than that it looks good.
npendery
left a comment
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!
1884764 to
7343508
Compare
What's in this PR?
Brings in some changes to the project and the sidebar.