Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Add Tabzilla #13

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
3 participants
Member

tallowen commented Mar 18, 2013

Work in progress.

Things that changed:

  • Old background header/footer removed
  • Using a gradient background instead of an image since I think the pinstripes are out.
  • Using Tabzilla now
  • Using Open Sans

Things that still need to change:

  • Footer needs to come from somewhere... should I strait up copy it from bedrock?
  • The links should be changed. The download button is a picture and should be styled. The bottom border looks dumb on the last row of elements.
  • The header image needs fixing - it either needs to be re-sized with the page or it should not be

Full Screen:

Full Screen

Narrow View

Narrow View

@tallowen tallowen commented on the diff Mar 18, 2013

templates/builds.html
@@ -0,0 +1,32 @@
+{% for group in files %}
+ <div id="{{ group['name'] }}" class="build-group">
+ <h3>
+ {{ group['name'] }}
+ {% if group['subtitle'] %}
+ <small>{{ group['subtitle'] }}</small>
+ {% endif %}
+ </h3>
+
+ <ul>
@tallowen

tallowen Mar 18, 2013

Member

I feel like this section could be done in a more effective way

@tofumatt

tofumatt Apr 2, 2013

Member

It doesn't really matter, given that this just outputs HTML. So if you can think of one you can, but I don't really care since it's all static in the end.

@ghost ghost assigned tofumatt Mar 18, 2013

Member

tofumatt commented Mar 27, 2013

I just got in from Ecuador so I'm a bit scatterbrained; also: I'm in a crunch mode on some Q1 goals that I want to finish within the next week, but I will give you feedback on this ASAP! Worst-case I'll respond by Sunday. Sorry for the delay!

Member

tallowen commented Mar 27, 2013

No worries. I won't be back to internet until next week anyway.
On Mar 26, 2013 11:52 PM, "Matthew Riley MacPherson" <
notifications@github.com> wrote:

I just got in from Ecuador so I'm a bit scatterbrained; also: I'm in a
crunch mode on some Q1 goals that I want to finish within the next week,
but I will give you feedback on this ASAP! Worst-case I'll respond by
Sunday. Sorry for the delay!


Reply to this email directly or view it on GitHubhttps://github.com/mozilla/nocturnal/pull/13#issuecomment-15501552
.

@tofumatt tofumatt commented on the diff Apr 2, 2013

templates/builds.html
+ {{ group['name'] }}
+ {% if group['subtitle'] %}
+ <small>{{ group['subtitle'] }}</small>
+ {% endif %}
+ </h3>
+
+ <ul>
+ {% set previous_build_name = '' %}
+
+ {% for build in group['builds'] %}
+ {% if build['link'] %}
+ <li class="{{ build['class'] }}
+ {{ 'first' if previous_build_name != build['class'] }}">
+ <a href="{{ build['link'] }}">
+ {{ build['name'] }}
+ <small>{{ build['size'] }}B | {{ build['extension'] }}</small>
@tofumatt

tofumatt Apr 2, 2013

Member

Outputing the size in bytes?

@tallowen

tallowen Apr 3, 2013

Member

Hmm - thats what was going on before. https://github.com/mozilla/nocturnal/pull/13/files#L15L54

After more investigation (a print statement) it looks like the scraping script is just not putting the 'B' on the end of the extension:
sizes in mb

@tofumatt

tofumatt Apr 17, 2013

Member

Ah, I get it. Yeah, the current site adds "B" which just makes things look nice:
Screen Shot 2013-04-17 at 4 47 35 PM

@tofumatt tofumatt commented on an outdated diff Apr 2, 2013

templates/index.html
<meta name="og:image"
content="http://mozcom-cdn.mozilla.net/img/firefox-100.jpg">
<title>Firefox Nightly Builds</title>
+ <!--[if lt IE 9]>
+ <script src="http://html5shim.googlecode.com/svn/trunk/html5.js"></script>
@tofumatt

tofumatt Apr 2, 2013

Member

Nit: Use two spaces for indentation here, not four.

@tofumatt tofumatt commented on an outdated diff Apr 2, 2013

templates/index.html
<link rel="icon" type="image/png" href="img/favicon.png">
+ <link rel="stylesheet" media="all" href="css/nightly.css">
@tofumatt

tofumatt Apr 2, 2013

Member

media="all" is redundant, no? (Or rather, it's implicit so it's not needed.)

@tofumatt tofumatt commented on the diff Apr 2, 2013

reloader.py
+
+class FileSystemChangeHandler(FileSystemEventHandler):
+ def on_any_event(self, event):
+ if not event.src_path.startswith(os.path.join(CURRENT_PATH, OUTPUT_PATH)):
+ print 'Reloading'
+ try:
+ main()
+ except (TemplateSyntaxError, UnicodeEncodeError) as e:
+ print e
+ else:
+ print 'Reloaded'
+
+
+if __name__ == "__main__":
+ # Initial Run
+ print 'Starting initial build (make sure you have internet).'

@tofumatt tofumatt commented on the diff Apr 2, 2013

reloader.py
@@ -0,0 +1,39 @@
@tofumatt

tofumatt Apr 2, 2013

Member

Good idea!

@tofumatt tofumatt commented on an outdated diff Apr 2, 2013

@@ -0,0 +1,39 @@
+import time
+import os
@tofumatt

tofumatt Apr 2, 2013

Member

Should come before import time. (alphabetize)

@tofumatt tofumatt commented on an outdated diff Apr 2, 2013

@@ -0,0 +1,39 @@
+import time
+import os
+
+from watchdog.observers import Observer
+from watchdog.events import FileSystemEventHandler
@tofumatt

tofumatt Apr 2, 2013

Member

alphabetize

@tofumatt tofumatt commented on an outdated diff Apr 2, 2013

@@ -0,0 +1,39 @@
+import time
+import os
+
+from watchdog.observers import Observer
+from watchdog.events import FileSystemEventHandler
+
+from scrape import main, CURRENT_PATH, OUTPUT_PATH
@tofumatt

tofumatt Apr 2, 2013

Member

Constants should come first.

@tofumatt tofumatt and 1 other commented on an outdated diff Apr 2, 2013

css/nightly.css
+ #nightly-title {
+ background: url(../img/nightly-header-bg.png) -65px -75px no-repeat;
+ padding: 65px 0 55px;
+ }
+
+ .build-group li {
+ width: 100%;
+ }
+
+ .build-group h3, .build-group ul {
+ float: none;
+ overflow: auto;
+ }
+
+ .build-group h3 {
+ height: auto !important;
@tofumatt

tofumatt Apr 2, 2013

Member

Is this my fault or did you add the !important?

@tallowen

tallowen Apr 3, 2013

Member

The important is there to overide this action: https://github.com/mozilla/nocturnal/blob/master/js/nightly.js#L6

jQuery is putting the style on element so it needs to be override with important... perhaps there is a better way to set the height of h3 because it looks like this doesn't work when you change the window size.

@tofumatt

tofumatt Apr 17, 2013

Member

Fair enough... I'll say this is good for now, but maybe you can add a comment as to why you're doing it... and thees thoughts on possibly making it better in the future?

Short answer: it's @tofumatt's fault.

@tofumatt tofumatt commented on an outdated diff Apr 2, 2013

css/nightly.css
@@ -189,7 +105,7 @@ body {
-moz-box-shadow: inset 1px 1px 8px 1px #1b335b;
box-shadow: inset 1px 1px 8px 1px #1b335b;
float: left;
- font: normal 20px 'MuseoSans500', sans-serif;
+ font-size: normal 20px;
@tofumatt

tofumatt Apr 2, 2013

Member

Two values in font-size? What's the "normal" do?

@tofumatt tofumatt commented on an outdated diff Apr 2, 2013

css/nightly.css
}
-
-#header {
- background: none;
- border: 0;
- border-radius: 0;
- box-shadow: none;
- height: 57px;
- margin: 0 auto;
- width: 970px;
+.small, small {
@tofumatt

tofumatt Apr 2, 2013

Member

Nit: use four spaces for CSS indents here and above.

Member

tofumatt commented Apr 2, 2013

This is one of the prettiest pull requests I've seen in awhile, btw.

Member

tofumatt commented Apr 2, 2013

Unless there's a good way to pull in the Mozilla footer (@Osmose @craigcook @pmclanahan?), then just copying is fine by me.

Member

tofumatt commented Apr 2, 2013

I'll have to ask Sean about the header...

Member

tofumatt commented Apr 2, 2013

Most of this is awesome. Wanna change up some of the nits and I'll see about your questions/remaining bugs?

Freaking sweet though. Nice work dude.

Member

pmclanahan commented Apr 2, 2013

I don't know of a Moz footer that can be externally included. Just steal and modify :)

Member

tallowen commented Apr 3, 2013

@tofumatt I think I've addressed all the issues you've brought up. I'll work on swiping the bedrock footer. I notice that its all in less. Is that something that is worth using here? I feel like it might help clean up a bit of the css...

Member

tofumatt commented Apr 17, 2013

If you want to use LESS I'm cool with that. You'll have to add it to the build step, but I'm totally for using LESS over stock CSS.

@tofumatt tofumatt commented on an outdated diff Apr 17, 2013

css/nightly.css
}
-
-::-webkit-selection {
- background: rgba(0, 0, 0, .95);
+h4 {
+ font-size: 24px;
+ letter-spacing: -0.25px;
+ line-height: 100%;
@tofumatt

tofumatt Apr 17, 2013

Member

I'd say you could have one rule that just does:

h1, h2, h3, h4, .large, .small {
    line-height: 100%;
}

Not sure if it's more efficient, but it's certainly more centralized, which I think is nicer here.

This is totally a nit thought.

@tofumatt tofumatt commented on an outdated diff Apr 17, 2013

css/nightly.css
@@ -148,7 +58,13 @@ body {
}
#main-content {
- background: url(../img/nightly-content-bg.png) 50% -40px no-repeat;
+ position: relative;
+ border-top: 2px solid rgb(255, 255, 255);
+
+ background: url(../img/nightly-content-bg.png) 50% -40px no-repeat, -webkit-linear-gradient(top, #002147, #000000);
@tofumatt

tofumatt Apr 17, 2013

Member

Nit: Use #000 instead of #000000.

@tofumatt tofumatt commented on an outdated diff Apr 17, 2013

css/nightly.css
@@ -148,7 +58,13 @@ body {
}
#main-content {
- background: url(../img/nightly-content-bg.png) 50% -40px no-repeat;
+ position: relative;
+ border-top: 2px solid rgb(255, 255, 255);
+
+ background: url(../img/nightly-content-bg.png) 50% -40px no-repeat, -webkit-linear-gradient(top, #002147, #000000);
+ background: url(../img/nightly-content-bg.png) 50% -40px no-repeat, -moz-linear-gradient(top, #002147, #000000);
+ background: url(../img/nightly-content-bg.png) 50% -40px no-repeat, linear-gradient(to bottom, #002147, #000000);
+
@tofumatt

tofumatt Apr 17, 2013

Member

Remove newline.

@tofumatt tofumatt commented on an outdated diff Apr 17, 2013

css/nightly.css
@@ -148,7 +58,13 @@ body {
}
#main-content {
- background: url(../img/nightly-content-bg.png) 50% -40px no-repeat;
+ position: relative;
+ border-top: 2px solid rgb(255, 255, 255);
+
@tofumatt

tofumatt Apr 17, 2013

Member

Unneeded newline.

@tofumatt tofumatt commented on an outdated diff Apr 17, 2013

css/nightly.css
@@ -349,3 +265,37 @@ body {
text-decoration: underline;
text-shadow: #404040 1px 1px 1px;
}
+
+@media only screen and (max-width:960px) {
@tofumatt

tofumatt Apr 17, 2013

Member

Nit: I like a space after max-width:.

@tofumatt tofumatt commented on an outdated diff Apr 17, 2013

css/nightly.css
@@ -349,3 +265,37 @@ body {
text-decoration: underline;
text-shadow: #404040 1px 1px 1px;
}
+
+@media only screen and (max-width:960px) {
+ #nightly-title, #nightly-links, #builds, #sub-footer #sub-footer-contents,
+ .build-group h3, .build-group ul, p#warning {
@tofumatt

tofumatt Apr 17, 2013

Member

You shouldn't need the p in the p#warning selector, I should think.

@tofumatt tofumatt commented on an outdated diff Apr 17, 2013

+import time
+
+from watchdog.observers import Observer
+from watchdog.events import FileSystemEventHandler
+
+from scrape import CURRENT_PATH, OUTPUT_PATH, main
+
+from jinja2 import TemplateSyntaxError
+
+
+class FileSystemChangeHandler(FileSystemEventHandler):
+ def on_any_event(self, event):
+ """
+ Handler for any file system change.
+
+ Runs more often than probably nessasary, but reloading is cheap since we have
@tofumatt

tofumatt Apr 17, 2013

Member

Thanks, this is even a nice explanation as to why we're inefficient in resource usage to many our code nice and lightweight.

@tofumatt tofumatt commented on the diff Apr 17, 2013

scrape.py
@@ -195,6 +195,11 @@ def main():
for group in files:
for build in group['builds']:
+ if 'date' in build and 'link' in build and 'size' in build:
@tofumatt

tofumatt Apr 17, 2013

Member

Python should totally have a better way to test for multiple items in a list. I'm surprised it doesn't?

@tallowen

tallowen Apr 22, 2013

Member

Could do something like

if all(t in build for t in {'date', 'link', 'size'}):

But I don't think that's more clear

@tofumatt tofumatt and 1 other commented on an outdated diff Apr 17, 2013

templates/index.html
<meta name="og:image"
content="http://mozcom-cdn.mozilla.net/img/firefox-100.jpg">
<title>Firefox Nightly Builds</title>
+ <!--[if lt IE 9]>
+ <script src="http://html5shim.googlecode.com/svn/trunk/html5.js"></script>
@tofumatt

tofumatt Apr 17, 2013

Member

Use // in case the site is loaded via HTTPS or we'll get mixed content warnings.

@tallowen

tallowen Apr 22, 2013

Member

Isn't there a specific issue with IE and using // wouldn't it be better to use https://?

@tofumatt tofumatt commented on an outdated diff Apr 17, 2013

templates/index.html
@@ -2,159 +2,46 @@
<html class="no-js" lang="en-US" dir="ltr">
<head>
<meta charset="utf-8">
- <meta name="viewport" content="width=1024">
<meta name="og:image"
content="http://mozcom-cdn.mozilla.net/img/firefox-100.jpg">
@tofumatt

tofumatt Apr 17, 2013

Member

I know it's not your fault, but could you change this (and anything else) to be protocol-relative so we don't get mixed content warnings? Merci beaucoup.

@tofumatt tofumatt commented on the diff Apr 17, 2013

templates/index.html
<link rel="icon" type="image/png" href="img/favicon.png">
+ <link rel="stylesheet" href="css/nightly.css">
+ <link href="//www.mozilla.org/tabzilla/media/css/tabzilla.css" rel="stylesheet">
@tofumatt

tofumatt Apr 17, 2013

Member

Very nice.

@tofumatt tofumatt commented on the diff Apr 17, 2013

templates/nightly-links.html
@@ -0,0 +1,42 @@
@tofumatt

tofumatt Apr 17, 2013

Member

I'll trust you that these are up-to-date with the links that are in HEAD, but please double-check? I recall one of the more recent pull requests I merged including changes to these links and I'm not sure if your branch would have been from code that recent. (Ignore me if you've rebased master in and I'm being nutso).

@tofumatt tofumatt commented on the diff Apr 17, 2013

templates/nightly-links.html
+<section>
+ <h3>Useful Add-ons</h3>
+ <ul>
+ <li><a href="https://addons.mozilla.org/firefox/addon/nightly-tester-tools/">Nightly Tester Tools</a></li>
+ <li><a href="https://addons.mozilla.org/firefox/addon/mozilla-qa-companion/">QA Companion</a></li>
+ <li><a href="https://addons.mozilla.org/firefox/addon/add-on-compatibility-reporter/">Add-on Compatibility Reporter</a></li>
+ <li><a href="https://addons.mozilla.org/firefox/addon/mozmill/">MozMill</a></li>
+ </ul>
+</section>
+
+<section>
+ <h3>Other Resources</h3>
+ <ul>
+ <li><a href="http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=48+hours+ago&amp;enddate=now">Last 48 hours of check-ins</a></li>
+ <li><a
+ href="http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly">Other Nightly Builds (FTP)</a></li>
@tofumatt

tofumatt Apr 17, 2013

Member

Meh, these are all over 80 chars. If you wanna let this me one line I'm okay with it.

Member

tofumatt commented Apr 17, 2013

So if this includes the footer and other TODOs I'd say this is r+wc.

Regarding the header image shinking... I was told by our very own Sean Martell that the image was better to use than text because it's gotta be pixel perfect. So I'm not sure about how to shrink it down. I will email him this thread and ask.

Member

tofumatt commented Apr 17, 2013

Oh! One other thing: this is optional, but it might be nice if the Android/mobile versions appeared above Windows on smaller screen sizes. I'd assume most of the hits to this site on mobile would be Android users looking to fetch a version for their phone, so if we can make it at all easier that'd be cool.

But if you can't do that with just CSS, which could well be a PITA, forget it 😄

Member

tallowen commented Apr 22, 2013

RE: android version having a button

I made an issue! #18

Member

tallowen commented May 16, 2013

@tofumatt Ever get a response from smartell?

Member

tofumatt commented May 16, 2013

Not yet, I'll bug again. Thanks for the ping.

Member

tallowen commented Sep 2, 2013

Should we land this before it gets super out of date? Is the bad graphic a deal breaker? @tofumatt were you ever able to talk to smartell?

Member

pmclanahan commented Feb 24, 2014

Bump?

Member

tallowen commented Feb 28, 2014

ohai! I cant really remember what was wrong with this.

@tallowen tallowen closed this Sep 26, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment