dailybruin / caeruleum

Daily Bruin's WordPress theme and special projects (2.0)
http://dailybruin.com
8 stars 15 forks source link

Adds the Blog-List Cards and Sidebar #100

Closed dustinnewman closed 7 years ago

dustinnewman commented 7 years ago

Fixes Issue #94

  1. If you resize the window vertically, the sidebar will go below the card

  2. If the post is smaller than the sidebar, the sidebar does not resize accordingly

Demo GIF: m7xib4izq0

nathanmsmith commented 7 years ago

Awesome @dustinnewman98! Just a couple of PR things:

Thanks for all your work! I'm really happy with what we've got right now 😊

nathanmsmith commented 7 years ago

Everything looks good to me! Thanks to @dustinnewman98 @binerys @tanzeelak and @ashleyshine! You're all amazing! @H-Huang, you think this is ready to go?

nathanmsmith commented 7 years ago

Mmm css/list-style.css was the original css file @tanzeelak and @ashleyshine wrote for the static version of the blog lists. I copied that all over into scss/app.scss, we should be able to just remove it. css/main.css appears to be some HTML5 Boilerplate code, which is probably left over from when Byron created Gryphondor from Roots. Not sure if using a HTML5 Boilerplate class was intentional or not though, perhaps @ashleyshine or @tanzeelak could answer?

tanzeelak commented 7 years ago

sidebar1 was created when testing with different versions of a sidebar for the bloglist template. We ended up never changing the name of the class, and were weary of changing it later on. The scss/app.scss file has another class called sidebar that is separate from the one we made.

nathanmsmith commented 7 years ago

Given that, I just renamed .sidebar1 to .blog-list-sidebar and everything seems to be working the same. Are we ready to merge?

H-Huang commented 7 years ago

bloglists

I tried it on our production servers and its looking like this. Any ideas?

nathanmsmith commented 7 years ago

Sizing can get weird when there isn't much content. Try adding a few paragraphs of lorem ipsum.

H-Huang commented 7 years ago

cool it looks fine that then. Couple more questions:

  1. is there any way to fix the issue where it has very little text?
  2. can we change it so it does not reset the page view to the top whenever you hit "last" or "next"
  3. is there some sort of CSS conflict with our related posts because they look like this now: image
nathanmsmith commented 7 years ago
  1. Yeah we can fix the little text issue. How do you want us to handle that design-wise? Should we:

    • remove the sidebar completely if the article isn't a certain length
    • vertically shrink the sidebar to the height of (or less than) the article if the article isn't a certain length
    • something else?
  2. From a reading perspective, I think it makes more sense for the article to go to the beginning when a user clicks "next" or "previous". (This is how Vox does it.) We can change if you want though.

  3. Did not notice that! Will look into.

H-Huang commented 7 years ago
  1. maybe make the card a minimum size? (just as long as it doesn't overflow over the footer anything will be fine I think).

  2. oh yeah you are right, its fine as it is.

H-Huang commented 7 years ago

@nathunsmitty hows the progress on this

nathanmsmith commented 7 years ago

Sorry--got busy with midterms last week and didn't get around to working on this. Just pushed the article height fix.

nathanmsmith commented 7 years ago

As for related posts, I'm not able to see them on any article I have in my virtual machine. When did we add related posts? My assumption is that we don't have related post data for articles from 2013, because articles from 2013 show an empty related post box on the site.

screen shot 2017-04-30 at 4 30 06 pm No related posts on article locally.

screen shot 2017-04-30 at 4 29 54 pm Empty related posts box on dailybruin.com for same article from 2013.

screen shot 2017-04-30 at 4 31 46 pm Perfectly fine related posts on article from 2016.

Tangentially related to this issue, if we don't have any related posts for an article, we probably shouldn't show a box at all. (This issue goes hand in hand with #102)

nathanmsmith commented 7 years ago

Alright should be fixed now--just an incorrectly placed close brace lol.