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

Layout Fixes #486

Merged
merged 8 commits into from
Aug 25, 2015
Merged

Conversation

caneruguz
Copy link
Contributor

Purpose

Fixes these two issues related to small screens:
#485
#484

and makes layout changes in News page that may be resolving other things as well.

Changes

News page:
This page was using tables for layouts and wrong nesting of rows and columns

  • unnecessary row-col structure removed
  • installed masonry plugin to avoid fixed heights that made the page look bad and cut some text that went beyond the fixed height.
  • restructured the boxes to work with the masonry plugin
  • CSS fixes

Communities Page

  • fixed wrong implementation of row-col structure

Our board Page

  • fixed wrong implementation of row-col structure

    Side Effects

    Shouldn't be any but news page in different layouts should be checked again. There is now a loader on search page. This is required for all images to load before initializing masonry. If this becomes

Screenshot of News Page

screen shot 2015-08-25 at 11 49 42 am

Other notes about masonry

Masonry library allows for more options. One of the most important for us is that we can have news items to span 2 or 3 columns. To achieve this add either .service-box-v1-col2 or .service-box-v1-col3 classes into the existing .service-box-v1 div like so:

    <div class="service-box-v1 service-box-v1-col2 ">

    </div>

Masonry can't guarantee bottoms of divs will align in this case so you need to choose where to use this.

Also I'm seeing a lot of wrong use of row and col layout structure in this repo. This reading may help: http://getbootstrap.com/css/#grid

erinspace added a commit that referenced this pull request Aug 25, 2015
@erinspace erinspace merged commit 49b6213 into CenterForOpenScience:master Aug 25, 2015
@billyhunt
Copy link

Looks good to me.

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.

3 participants