Skip to content
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

Update Header #79

Merged
merged 13 commits into from
Aug 11, 2017
Merged

Update Header #79

merged 13 commits into from
Aug 11, 2017

Conversation

athai
Copy link
Contributor

@athai athai commented May 25, 2017

Note, there are some issues that I couldn't figure out/resolve:

  • The text for parent menus (i.e. About) will only be white if you're hovering over them
  • There's a gap between the main menu and the submenu
  • How should the search bar be handled?
  • The font weight for the CASS heading is slightly off
  • The font used for the article headings (in the slideshow) needs to be obtained from OSU

@kelnera
Copy link

kelnera commented May 25, 2017

@athai Thanks for your hard work! I can only answer a couple of these concerns. The CASS website will quickly be back on drupal (probably within the next week) so don't worry about issues with that site. Just make sure things look good for the OSL website. If you go to #78 there's a link to the branding pages and on the font page, they have alternative options for all three new fonts. I'll work on getting the official ones, but this should suffice for now.

@ramereth
Copy link
Member

@athai looks like a great start! A few comments/answers

  • I would try implementing search similar to how the eecs website does where it shows a search icon and then pops up a frame. If that's not easy to do then I'd recommend we just keep it as-is.
  • The title heading for "OSU Open Source Lab" should be bold from what I can see on the EECS website
  • This might be related to needing the fonts, but the fonts for the website itself seem to not be using the correct font for the body and headings.
  • Looks like the new theme includes a background for the footer, we should probably try adding that
  • When you look at the site in a mobile size, there seems to be some lingering CSS issues:
    screenshot from 2017-05-31 10-03-41
    .

@Kennric Kennric self-assigned this Jul 14, 2017
@Kennric
Copy link
Contributor

Kennric commented Jul 25, 2017

@ramereth you can see the results of my changes here: http://osuosl-pelican-175.staging.osuosl.org

@Kennric Kennric requested review from ramereth and kelnera July 25, 2017 21:26
@kelnera
Copy link

kelnera commented Jul 25, 2017

Great progress! I have a few more things I noticed:

  • The titles of each page and big headers (like the headers for the blog feed) should also use the Impact font. It looks like most of the other font uses are more up in the air.

  • This might be because we're using the substitute fonts, but I noticed the arrows between CASS and The Open Source Lab in the black bar are bigger and farther apart than on the rest of the OSU sites.

  • Compare the OSL top hat orange with the OSU top hat orange; I don't think it's the same.

  • When you shrink the window way down, the orange gets even darker...

  • ...then when you make the window big again, the slideshow doesn't load properly.

Hopefully most of these are easy changes, although that last one might take more time so I won't ask that be fixed before we merge these changes; just something to keep an eye out for while you'll already be in the theme. Getting closer!

@Kennric
Copy link
Contributor

Kennric commented Jul 26, 2017

@ramereth, @kelnera I've updated a couple of things, the color and >> character, but the font EECS is using is a commercial font we don't want to add to the repo. I have not figured out the slideshow breakage on resize yet, and it might not be easily fixable - also it's a fairly edge case, I don't think users frequently resize while looking at the front page.

http://osuosl-pelican-175.staging.osuosl.org

@kelnera
Copy link

kelnera commented Jul 27, 2017

@Kennric Looks better and I think we're close. I knew the new fonts were specifically for OSU and they're not accessible to the public (or easy to download) so it's nice they've offered the alternative fonts. I think the only thing I would like to change before merging is applying the Impact font (alternative to Stratum 2) to the page headers and the blog feed. But I also know we're on a time crunch so if we need to push as is we can just make an issue.

@ramereth
Copy link
Member

@Kennric I think this is basically ready for desktops, but on mobile or responsive it seems to have a few issues specifically in the header section. There also seems to be quite a bit of white space from the image banner to the text below:

screenshot from 2017-07-27 08-48-00

In comparison, the EECS website looks like this:

screenshot from 2017-07-27 08-49-41

@ramereth
Copy link
Member

ramereth commented Aug 1, 2017

@JerryPeng0112 please feel free to merge this and then create a new PR which merges develop into master.

@ramereth ramereth merged commit d9972fb into develop Aug 11, 2017
@ramereth ramereth deleted the thai/update_header branch August 11, 2017 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants