lonelyplanet/ui-test — pull request #28 by CIPowell (2014-02-01 12:51:20+00:00)
Title: Submission

HAML:

I have added sections to the index page to enclose each food type and the images at the bottom. I've used section where there are divisions in the content, and left divs where they're included for presentation reasons. Also, then a section could be manipulated independently by js (say in the future if some sort of filtering was wanted)

I'd argue that the

element could also be appropriate, but consensus seems to be that an

is a syndicated or item such as a blog post.

SASS:

I've re-arranged the elements to give the document more structure in the sense of making it easier to find rules. Basically organising the rules, so that rules for tags are first, followed by classes, with both sections in alphabetical order. I've also removed (well commented out) some redundant rules, there's probably a few more that could go as well.

CoffeeScript:

First time I've ever used CoffeeScript! Bit of a headache to start with. I've split both functions down into smaller, more reusable blocks, and shifted the height values into the SASS file so all the presentation-level stuff is in the SASS file... and to use a css transition to show off. The good bit about this is that to test the JS we don't need to measure elements. As I've mentioned in comments there are a couple of points where it might not be the best from a PE point of view, firstly that the transition won't work in several browsers (and needs a prefix in others, but on a production project I'd use a mixin). Also I've isolated the close_with_keypress handler so it can be user elsewhere as long as it's bound to the correct element. My last point I'd make with the CoffeeScript is that I'd put some sort of close button on the lightbox. From experience the Esc key and the return key aren't a natural thought for many people and a visual convention like a close button is.