Skip to content

Commit

Permalink
Align the contents in the Filter box (mozilla#90)
Browse files Browse the repository at this point in the history
  • Loading branch information
@tojon
tojon committed May 16, 2014
1 parent 91fd612 commit 3801fcd
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 28 deletions.
25 changes: 21 additions & 4 deletions _attachments/css/main.css
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,15 +111,12 @@ html ul.topnav li ul.subnav li a:hover {
clear: both;
}

/* Filter styling */
#filter {
width: 620px;
text-align: left;
}

#filter div {
padding: 5px 10px;
}

#filter div span {
padding: 2px 5px;
cursor: pointer;
Expand All @@ -132,6 +129,26 @@ html ul.topnav li ul.subnav li a:hover {
text-decoration: none;
}

.filterrow {
display: table-row;
}

.filtercell {
display: table-cell;
}

#filterupper div {
padding: 5px 5px;
}

#filterlower div {
padding: 2px 5px;
}

#filterdatespacer {
width: 23px;
}

#content #navigation {
margin: 0 35px 10px;
text-align: right;
Expand Down
53 changes: 29 additions & 24 deletions _attachments/templates/functional_reports.mustache
View file
Original file line number Diff line number Diff line change
@@ -1,34 +1,39 @@
<fieldset id="filter">
<legend>Filter</legend>

<div id="app-selection">
<span>Application: </span>
<span>All</span>
<span>Firefox</span>
<span>MetroFirefox</span>
</div>
<div id="filterupper">
<div class="filterrow">
<div class="filtercell"><span>Application:</span></div>
<div class="filtercell"><span class="selected">All</span></div>
<span>Firefox</span>
<span>MetroFirefox</span>
</div>

<div id="branch-selection">
<span>Branch:</span>
<span>All</span>
{{#firefox_versions}}
<span>{{{.}}}</span>
{{/firefox_versions}}
</div>
<div class="filterrow">
<div class="filtercell"><span>Branch:</span></div>
<div class="filtercell"><span class="selected">All</span></div>
{{#firefox_versions}}
<span>{{{.}}}</span>
{{/firefox_versions}}
</div>

<div id="os-selection">
<span>OS: </span>
<span>All</span>
<span>Linux</span>
<span>Mac</span>
<span>Win</span>
<div class="filterrow">
<div class="filtercell"><span>OS:</span></div>
<div class="filtercell"><span class="selected">All</span></div>
<span>Linux</span>
<span>Mac</span>
<span>Win</span>
</div>
</div>

<div id="date">
<span>From: </span>
<input id="start-date" class="datepicker" type="text" value="" size="11" />
<span>To: </span>
<input id="end-date" class="datepicker" type="text" value="" size="11" />
<div id="filterlower">
<div class="filterrow">
<div class="filtercell"><span>From:</span></div>
<div id="filterdatespacer"></div>
<div class="filtercell"><input id="start-date" class="datepicker" type="text" value="" size="11" /></div>
<span>To: </span>
<input id="end-date" class="datepicker" type="text" value="" size="11" />
</div>
</div>
</fieldset>

Expand Down

6 comments on commit 3801fcd

@tojon
Copy link
Owner Author

@tojon tojon commented on 3801fcd May 16, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andreieftimie here's the preliminary work for one template. I left the class="selected" for All in just for now for visual reference during the alignment work. It will be removed in an eventual PR. I also wasn't sure if the filtercell spans each need new lines, but a W3 check of the code didn't complain about that all being in one line. It seems easier to read.

@andreieftimie
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately this app's markup is heavily tied into the business logic. The changes you made here break all functionality around the filters.

That's why I would propose to not change the markup (otherwise you'll need a lot of changes in JS).
We can achieve the same effect using only CSS.

Something like the following should work (might need a bit of tweaking, but its pretty close).
Just float the left span and clear the enclosing divs. No markup changes required.

#filter div {
  padding: 3px 0;
}
#filter div:after {
  display: block;
  clear: both;
  height: 0;
  visibility: hidden;
  content: ' ';
}
#filter span:first-child {
  float: left;
  width: 100px;
}

@tojon
Copy link
Owner Author

@tojon tojon commented on 3801fcd May 20, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, crazy. Yes, I had tried just css only first, but didn't get the desired result. I will try your suggestions.

@tojon
Copy link
Owner Author

@tojon tojon commented on 3801fcd May 20, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I think you should be credited wholly with this fix :) Minor thing, I am trying to figure out why the text baselines are not aligned across all four rows. The float: left seems to trigger it. It pushes each of the four first-child spans (Application, Branch, OS, From) downwards.

baseline

@andreieftimie
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's because of the vertical padding applied to the span elements which are inline. Padding doesn't really work for inline elements.

We can use display: inline-block to mitigate this. Even better is probably to not rely on vertical padding, just use line-height to make sure all our text elements render with the same leading.

I've pushed a result to http://mozmill-sandbox.blargon7.com/

@tojon
Copy link
Owner Author

@tojon tojon commented on 3801fcd May 22, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks cool to me! I've made one comment in the PR. Thank you for helping out and explaining the above Andrei, appreciated.

Please sign in to comment.