-
Notifications
You must be signed in to change notification settings - Fork 14.7k
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
Add new case study stylesheet #17334
Add new case study stylesheet #17334
Conversation
We will be adding a bunch of new case studies with an updated design (a lot of it is just simplified). I created this new file, and for the new case studies we will add this updated code at the top from now on: --- case_study_styles: true cid: caseStudies css: /css/new_case_studies.css --- So just replacing the old css file with this one. Let me know if that is not going to work the way I think it will, thank you!
/assign @bradtopol |
Deploy preview for kubernetes-io-master-staging ready! Built with commit 7b473eb https://deploy-preview-17334--kubernetes-io-master-staging.netlify.com |
A nit about names: If not, how about putting a date in place of “new”? |
@sftim thank you and sounds like a good idea. I appreciate the help. Definitely makes sense why that would get confusing, so I have renamed it to case_studies_20191031.css (date at the end) Let me know if that doesn't work! |
hello @alexcontini . How do the new style settings relate to the values set in assets/sass_case-studies.sass, assets/style.sass? |
@kbhawkey it's just an updated design for the case studies, so instead of having to use blue images, we can use black and white and there is a gradient overlay added. There are also a lot of spacing modifications, new stat section and the design is simplified overall. So there is nothing specific besides the fact that it's an updated design... Maybe we could call it case-studies-gradient or something like that? |
👍 |
@kbhawkey I updated the file name, please merge when you have a chance, thank you! |
static/css/new_case_studies.css
Outdated
|
||
.article * { | ||
position: relative; | ||
/* hack */ |
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.
hello @alexcontini . Is there additional follow up work to this PR?
It is hard to evaluate the styling, unless there is an example page using these settings. Just curious, how is line 40 used? Otherwise seems okay. Could use a space on:
line 25: .overlay:before{
line 331: @media screen and (max-width: 580px){
@kbhawkey - will make those updates for sure, but this is for a new set of case studies, so none of the current ones will be affected. Is there any way to merge once I make the changes, and then @jkld11 can add another PR with one of the new case studies so we can see how the new styling looks on the preview site? |
OK |
/unassign |
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.
These changes are fine by me
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jimangel The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
We will be adding a bunch of new case studies with an updated design (a lot of it is just simplified). I created this new file, and for the new case studies we will add
this updated code at the top from now on:
case_study_styles: true
cid: caseStudies
css: /css/new_case_studies.css
So just replacing the old css file with this one. Let me know if that is not going to work the way I think it will, thank you!
cc @jkld11