-
Notifications
You must be signed in to change notification settings - Fork 51
Pubsite modernization - for real this time #296
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
Pubsite modernization - for real this time #296
Conversation
|
This isn't quite ready for merge due to the issues below, but I will leave it open and update progress here.
|
|
Fixed those previous issues. Upon testing the blog page on mobile I discovered another bootstrap card conflict. |
|
All issues resolved, ready for review. |
|
@VivianNK Could you send some screenshots? |
|
Will do. |
|
Site available here: https://csh-public-site-new-dev-pubsite.apps.okd4.csh.rit.edu |
|
Note: both the 'Things we're working on' and the 'About us' sections have on:hover properties that don't come through on the desktop screenshots, but are disabled via mediaquery on mobile devices |
Dr-N0
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.
Just a few pedantic things, otherwise looks great!
Dr-N0
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.
👍
|
Thanks! Do you think I should change opacity of the slides over the project images? If so I will, I can't quite tell if it's too dark or not. |
I think those could be a little brighter, but it doesn't take away toooo too much so I'll leave it to whatever you think looks best (as long as it's not black lmao) Also ignore the close, my hand slipped on the comment button |
|
sounds good! I'll make them slightly lighter |
|
I think it would be cool to see if there's a good spot for https://youtube.com/watch?v=g9F-1vw0m-o on the site near drink. Maybe for another PR, but this PR made me think of that |
| </div> | ||
| </div> | ||
| <div class="stat_card"> | ||
| <p class="stat_big">1388</p> |
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 plan to eventually make this a live number?
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.
Yes, that number was pulled manually from ldap. I'm not quite ready to write that part, I want to get some help to make sure that I'm doing it in a secure manner.
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.
Makes sense. Odds are we can give the CI in OKD a service account it can use to make some queries during build.
I'll definitely look around and see where it could fit in, but that will be a separate PR. |
|
I'm going to clean up my commit history tommorow morning to make it easier to read and more consistent. We should probably wait to merge until then. |
Copied over my scss to main.scss and added stats Most of my changes are done Most of my changes are done
Updated gitignore to ignore .jekyll-cache Removed .jekyll-cache/
Updated index.html Update index.html
Making the project images slightly brighter Added stats to homepage and dropped in my scss Most of my changes are done Added stats to homepage and added my scss Revert "Added stats to homepage and dropped in my scss" This reverts commit ae92443.
|
Ok, the commit history is now clean and ready for merge. I can merge it, if you both think it looks good. I'll work on those other changes that were mentioned towards the bottom in future PRs. |
Dr-N0
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.
Just a couple of small css things and one bug. Otherwise solid
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.
Reviewed @Dr-N0 suggested changes, lgtm
Dr-N0
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.
👍



Very similar to my old re-write except I kept more of the original site intact, and just added my changes in places they fit. Images are now stored in s3. Site builds and runs in okd. Link to dev site: https://csh-public-site-new-dev-pubsite.apps.okd4.csh.rit.edu/