team178 / team178.github.io

The 2nd Law Enforcers' Website
http://farmingtonrobotics.org
Mozilla Public License 2.0
14 stars 17 forks source link

Create full width page layout #20

Closed gluxon closed 8 years ago

gluxon commented 9 years ago

The new team history page is very nice. Once thing that may improve it is another layout that sets #content > article to 100% width.

Just a suggestion for a quick task that may help someone with code literacy.

adetoudomkh commented 9 years ago

Made timeline full width and deleted history text b/c already on the timeline. Moved the content out of the article and deleted article altogether.

We are going to also make a version that changes the CSS. --Giselle

adetoudomkh commented 9 years ago

Added article back in and added a new CSS style. Here are all the changes I had made to fix this issue: ebba13b 80de26d 17ceb43

gluxon commented 9 years ago

I didn't understand that comment about deleting article. What was that about?

gluxon commented 9 years ago

Nice creative solution to creating the full page layout. It demonstrates understanding. However, something that could improve performance and maintainability would be to use CSS classes rather than external stylesheets.

  1. Move the contents of the file fullwidth.css to main.css (to preferable around line 90)
  2. Delete the now empty fullwidth.css
  3. Change #content article to #content article .fullwidth in the code you just copied
  4. Remove the external CSS link in fullpage.html
  5. Add class="fullwidth" to article.
gluxon commented 9 years ago

Another separate improvement that could be made is to set max-width rather than width.

#content article .fullwidth {
  width: 100%;
  max-width: none;
}
#content article .fullwidth {
  max-width: 100%;
}

If you're wondering "why is this better?", it's simply because this CSS is overriding a property that's already set. Best practice is to change just that property and not introduce another one.

gisellegk commented 9 years ago

Changing max-width. Darn, that's so obvious I didn't think of it, lol.

Instead of adding class=fullwidth to article, would it be possible to add it to only whatever's containing the html of the timeline (or whatever element we want to be fullwidth) ?

For example in the history page, the title and any content we might decide to have below the timeline would behave like they would in a default template page (would help with #26) but the timeline itself would span the entire page.

gisellegk commented 9 years ago

@gluxon deleting article was for demonstrating how css affects html, ie. full-width: 80% affects stuff in <article> but not in <div id="content">. Then we made the solution that is currently being used.

gisellegk commented 8 years ago

Does anyone know if this issue still needs to be open? @gluxon @adetoudomkh @meganlauren1211 @SilverSapo ~~ nvm I actually read the comments and there's some improvements we could do!

gluxon commented 8 years ago

Instead of adding class=fullwidth to article, would it be possible to add it to only whatever's containing the html of the timeline (or whatever element we want to be fullwidth)?

Yes, but that's slightly complicated. It would require either text paragraphs to have a min-width set on them, or the element that's full width to have negative margins. I recommend going with the comments above and then pursuing this afterwards.

nvm I actually read the comments and there's some improvements we could do!

:)