Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I've put together a board for Ultimate Tic-Tac-Toe (What's that?). This is part of the current : Code a Ultimate Tic-Tac-Toe

Resources:

Questions:

  • What are your thoughts on the Markup? Overkill? Just right?
  • What would you improve on the CSS side? Especially asking in the direction of absolute dimensions and borders.

HTML:

<div class="game">
    <div class="area area-0-0">
        <div class="tile tile-0-0">
            <button type="button" class="tile-button"></button>
        </div>
        <div class="tile tile-1-0">
            <button type="button" class="tile-button"></button>
        </div>
        <div class="tile tile-2-0">
            <button type="button" class="tile-button"></button>
        </div>
        <div class="tile tile-0-1">
            <button type="button" class="tile-button"></button>
        </div>
        <div class="tile tile-1-1">
            <button type="button" class="tile-button"></button>
        </div>
        <div class="tile tile-2-1">
            <button type="button" class="tile-button"></button>
        </div>
        <div class="tile tile-0-2">
            <button type="button" class="tile-button"></button>
        </div>
        <div class="tile tile-1-2">
            <button type="button" class="tile-button"></button>
        </div>
        <div class="tile tile-2-2">
            <button type="button" class="tile-button"></button>
        </div>
    </div>
    <div class="area area-1-0">
        <!-- 9 tiles -->
    </div>
    <div class="area area-2-0">
        <!-- 9 tiles -->
    </div>
    <div class="area area-0-1">
        <!-- 9 tiles -->
    </div>
    <div class="area area-1-1">
        <!-- 9 tiles -->
    </div>
    <div class="area area-2-1">
        <!-- 9 tiles -->
    </div>
    <div class="area area-0-2">
        <!-- 9 tiles -->
    </div>
    <div class="area area-1-2">
        <!-- 9 tiles -->
    </div>
    <div class="area area-2-2">
        <!-- 9 tiles -->
    </div>
</div>

CSS:

*, *:before, *:after {
    -webkit-box-sizing: border-box;
       -moz-box-sizing: border-box;
            box-sizing: border-box;
}

body {
    margin: 0;
}



/*
 *  GAME
 *  Board size is calculated as follows:
 *
 *  .game = .area * 3
 *  459px = 153px * 3
 *
 *  .area = padding-left + .tile * 3 + padding-right + border
 *  153px = 3px          + 48px  * 3 + 3px           + 3px
 *
 *  .tile = padding-left + .width    + padding-right
 *   48px = 3px          + 42px      + 3px
 */
.game {
    width: 459px;
    margin-top: 50px;
    margin-right: auto;
    margin-left: auto;
}

/* Clearfixing .game */
.game:after {
    content: "";
    display: table;
    clear: both;
}

@media screen and (max-width: 500px) {
    .game {
        width: 315px;
    }
}



/*
 *  AREA
 */
.area {
    float: left;
    width: 153px;
    padding: 3px;
}

.area:nth-child(3n-1),
.area:nth-child(3n-0) {
    border-left: 3px solid #444;
}

.area:nth-child(4),
.area:nth-child(5),
.area:nth-child(6),
.area:nth-child(7),
.area:nth-child(8),
.area:nth-child(9) {
    border-top: 3px solid #444;
}

/* Clearfixing .area */
.area:after {
    content: "";
    display: table;
    clear: both;
}

@media screen and (max-width: 500px) {
    .area {
        width: 105px;
    }
}



/*
 *  TILE
 */
.tile {
    float: left;
    padding: 3px;
}



/*
 *  BUTTONS
 */
.tile-button {
    width: 42px;
    height: 42px;
    padding: 0;
    cursor: pointer;
    vertical-align: middle;

    border: 1px solid gray;
    border-radius: 3px;
    background-color: greenYellow;
}

@media screen and (max-width: 500px) {
    .tile-button {
        width: 26px;
        height: 26px;
    }
}

.tile-button:hover,
.tile-button:focus {
    border-color: #444;
    background-color: yellowGreen;
}
share|improve this question
2  
+1 for referring to me, and thanks a lot for the help! –  Simon André Forsberg Feb 4 at 20:46
1  
@Simon Never thought I could somehow participate in the Code Challenges, but here we go. Good teamwork. Thank you as well. (Sorry for being off-topic) –  kleinfreund Feb 4 at 20:48
2  
Is the grid illusion of your live demo on purpose? ;) –  syb0rg Feb 4 at 22:43
    
@syb0rg No grid illusion for me. :p –  kleinfreund Feb 5 at 10:49
3  
Hey @kleinfreund, just wanted to let you know that you are missed. I hope you are doing well and that you will drop by our chat room again soon! –  Simon André Forsberg Apr 28 at 20:19
add comment

3 Answers 3

up vote 9 down vote accepted

It looks good. I like the way it adapts to smaller windows.

I'm not sure why (except as an aretfact of styling) you have a div around each button; maybe the button alone would do.

Instead of using divs it might be possible/appropriate to use tables instead.

You haven't demonstrated (although Simon's game does demonstrate) how you intend to mark played buttons with an O or X; nor how to draw a line through won games. I'd find it easier if X and O were different colors (e.g. blue and red to reflect the colors you chose for won games); and I don't like the font-size: small used in your clean.css.

To draw a (horizontal, vertical, or diagonal) line through won games, maybe make the buttons semi-transparent so that the line can be a CSS background.

When a quadrant is unplayable (because the opponent played elsewhere), do you still intend to draw buttons in that quadrant, or something else?

Do you need to do anything to adjust the apparent size on a 'retina' (double-density-pixel) display?

Does each button need a name attribute and should they be embedded in a form? Or will you be adding javascript?

Maybe make the index 1-based (1 through 9) instead of 0 based (0 through 8). Have you thought about accessibility (e.g. someone using a screen-reader)?

Maybe supply the game state in JSON as well as HTML, so that a bot that's playing (e.g. perhaps a Greasemonkey script) doesn't need to scrape the HTML to discover the game state.

Why are you using px instead of em or % to specify CSS dimensions?

share|improve this answer
    
I used an additional div around the control element (button in my version) to have better control over thinks like padding/margin. We discussed the table-topic in the chat. We want the table only for the easier layout handling. Therefor we wanted to use div's instead. Also, it's a Code Challenge. :p –  kleinfreund Feb 5 at 10:54
    
I'm only doing the view for Simon's working game, because I'm not a programmer. The clean.css you refer to is some generated stuff from Simon's game, not mine. I don't like it either. I will discuss your questions in the chat. Also we agreed on using 0 through 8 based indexing to mirror the logic. I'd prefer 1 through 9, tho. –  kleinfreund Feb 5 at 10:58
    
X and O in different colors, that I LIKE! Technically, the buttons require neither names nor specific CSS classes for the logic to work as I've made my implementation in GWT (Google Web Toolkit) and GWT keeps references to the buttons when they are added dynamically. I like the idea of JSON! I don't see what good 1-based indexes would make? –  Simon André Forsberg Feb 6 at 17:30
    
@SimonAndréForsberg 1-based is perhaps a dumb idea, for the human reader (if any) not for the browser. I can 'visualize' the location when it's only 0..2, but it's harder with 0..8. More important UI-related comments were IMO perhaps not using px, displaying lines (perhaps through translucent buttons). –  ChrisW Feb 7 at 0:39
add comment

The one thing I see here that's absolutely unnecessary is the use of the button element. If you're using event delegation properly (I can't tell by looking at the functional version since it's obfuscated), all you should need is a single event handler attached to the game board. Just check to see if your event's target element is a tile and make adjustments as necessary to the tile's attributes or child nodes.

Game boards are one areas where I believe that it is appropriate to use tables, even though it feels like you would be using them for layout. If you consider a game like Chess, the tiles are all named (A1-H8). The use of tables would actually simplify some of your CSS, since you would be able to make use of properties like border-spacing: 5px to get those nice gaps between your individual board tiles.

The naming conventions you've chosen for your tiles' class names leads me to believe that you're using JavaScript to parse their positions. I recommend using data-* attributes instead:

<div class="tile" data-row="1" data-col="1"></div>
<div class="tile" data-row="1" data-col="2"></div>

Here's a demo that illustrates some of these ideas for a card game (the interactivity is there, but the gameplay isn't): http://codepen.io/cimmanon/pen/lKgpJ

share|improve this answer
    
I'm doing the view for Simon's game, but not the logic. I'm not a programmer. Thank you for your suggestions. I will consider these. Sounds good to me. –  kleinfreund Feb 5 at 10:59
    
GWT (Google Web Toolkit) takes care of all the JavaScript stuff for me. It compiles my Java code to JavaScript. Therefore, your JS suggestion doesn't help me :) I like the idea of data-row instead of the class names though (technically I don't even need all the class names, as GWT automatically keeps references to the elements when they are added). As for the CSS, I let @kleinfreund decide on that :) –  Simon André Forsberg Feb 6 at 17:24
add comment

You can simplify this

.area:nth-child(4),
.area:nth-child(5),
.area:nth-child(6),
.area:nth-child(7),
.area:nth-child(8),
.area:nth-child(9) {
    border-top: 3px solid #444;
}

to

.area:nth-child(n + 4) {
    border-top: 3px solid #444;
}

Since you are calculating the dimensions according to the game, and later setting media-queries, it will make it more easy to set

area {
    width: 33%;
}

and 

tile {
    width: 28%;
    padding 2%;
} 

or something in this line. Then, you don't need to worry to get your queries in sync.

share|improve this answer
    
I was using display: inline-block; instead of float: left; in earlier versions and had problems with the the calculated dimensions on mobile devices. So for the first working demo I stuck to pixels. I will use percentages in the next version. Thank you. :) –  kleinfreund Feb 5 at 10:51
    
As for the multiple .area selectors, I agree that didn't look very optimal, I think I will apply specific class names such as has-top-border and has-left-border and apply those dynamically from my GWT (Google Web Toolkit) code. –  Simon André Forsberg Feb 6 at 17:26
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.