HiveBrain v1.2.0
Get Started
← Back to all entries
patternhtmlMinor

Social image sharing site with mobile-first, responsive layout

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
socialimagewithlayoutresponsivefirstsitemobilesharing

Problem

I am rebuilding the front-end for an application that I am building and have the content laid out the way I need.

I would appreciate any suggestions on how to improve the structure, specifically the header section) with web standards in mind.

You can view the Fiddle here. - edit please ignore the non loading icons in the navigation on jsfiddle.



`/* http://meyerweb.com/eric/tools/css/reset/
v2.0 | 20110126
License: none (public domain)
*/

html, body, div, span, applet, object, iframe,
h1, h2, h3, h4, h5, h6, p, blockquote, pre,
a, abbr, acronym, address, big, cite, code,
del, dfn, em, img, ins, kbd, q, s, samp,
small, strike, strong, sub, sup, tt, var,
b, u, i, center,
dl, dt, dd, ol, ul, li,
fieldset, form, label, legend,
table, caption, tbody, tfoot, thead, tr, th, td,
article, aside, canvas, details, embed,
figure, figcaption, footer, header, hgroup,
menu, nav, output, ruby, section, summary,
time, mark, audio, video {
margin: 0;
padding: 0;
border: 0;
font-size: 100%;
font: inherit;
vertical-align: baseline;
}
/ HTML5 display-role reset for older browsers /
article, aside, details, figcaption, figure,
footer, header, hgroup, menu, nav, section {
display: block;
}
body {
line-height: 1;
}
ol, ul {
list-style: none;
}
blockquote, q {
quotes: none;
}
blockquote:before, blockquote:after,
q:before, q:after {
content: '';
content: none;
}
table {
border-collapse: collapse;
border-spacing: 0;
}

/ Some CSS Setup - nothing to do with flexbox /
html { box-sizing: border-box; }
, :before, *:after { box-sizing: inherit; }
body { font-family: 'Source Sans Pro', sans-serif; font-size: 1.5em; font-weight: 100; margin: 0; }

body {
background: #FAFAFA;
}

header {
background: white;
}

header h1 {
font-weight: 400;
padding: 1em;
background: #5E35B1;
color: white;

Solution

Responsive

  • This is really nice with em and percentages being used effectively - get your head around that and media queries and that's responsiveness for screen sizes.



  • Media Queries are your friend and can allow you to re-organise things on smaller screens or different aspect ratios.



  • Responsive image load is a nice idea where you have several images with different widths and maintained aspect ratios, you then load them progressively wider and wider until the last one that fits inside the viewport. This means your user never loads an unnecessarily large image and doesn't have to wait for a huge image to load on a slow connection.



  • Remember that responsive design is a consideration of three main things: screen-size; bandwidth; and touchscreen (a finger is very different interface from a mouse pointer).



CSS

  • Don't use IDs - there's no need, a class can achieve anything an ID can without being restricted to one use on a page or imposing sledge-hammer specificity. More here



  • Don't use html entities in css, such as this .comment div - there are three reasons: firstly, you'll end up adding another tag into the parent and it will get unexpected styles; secondly, you won't be able to re-use the css without copying the HTML structure too; and thirdly, it introduces unnecessary specificity. Adding a class to the target is better (div in the one I picked out) and styling that directly is best (this applies to selectors in JS too)



  • You can put box-sizing: border-box; directly on the * selector rather than using html and inherit.



  • State classes can be identified by prefixing with is- so you'd have is-open. You can put these at the end of a style sheet and add them to anything without qualifying them (rather than .user-functions.open). You do need to be careful with specificity for this to work well.



  • Don't use overqualified tags, so span.top-posts is not good. .top-posts is better as it's re-usable without being put on a ` and won't break as easily when the mark-up changes.



HTML

  • tags are not quite right. I'd use an tag to cover the text and the comments, with s within that, including one for the comments and each comment as an . The spec here has a couple of examples of marking up blog posts. You may also want to consider a and tag in the article.



  • Add some aria-roles. For example, - this is good for any mechanical interpretation of your content, most crucially accessibility tools like screen readers. role="main" can go on your main article. You might think using a tag would be good here, but given the structure of article body with comments, a containing article tag is better. Here's a nice cheatsheet and here's the spec.



  • I'm guessing toggleNav is a js-hook. It's nice to differentiate them and to separate them from the css hooks, however, I prefer the js- prefix for this rather than CamelCase. This is preference really but is also common practice: all class attributes are snake-cased.



  • The list of links in the element should be organised as an unordered list like all the others contained within the tag. This is standard practice. You could also drop the and use the role described in point 2 on the element (incidentally, if you make this change, you'll notice why using html elements in css can be buggy!). This is a bit flexible, the key thing is probably to organise the links in a proper HTML list.



JavaScript

  • Nice to see add and remove class being used with a state class in JS.



-
You've passed in an anonymous function to the
click() method. You could use a re-usable function variable, like this:

var clickHandler = function() { / commands / }

$('.js-element').click(clickHandler);`

There are several good reasons for this, including that it's nicely named and re-usable. This article has more detail.

Context

StackExchange Code Review Q#113313, answer score: 2

Revisions (0)

No revisions yet.