Porting Calendar page to bedrock - take 1 #372

wants to merge 3 commits into


None yet
5 participants

milossh commented Sep 21, 2012

Porting Calendar page to bedrock. Please create a new calendar branch.

+ <div class="centerbox">
+ <div class="topbox row">
+ <div class="four columns logobox hide-for-touch">
+ <img src="{{ MEDIA_URL }}img/calendar/lightning/calendar-logo.png" border="0"/>

icaaq Sep 21, 2012

  1. border attribute is obsolete http://www.w3.org/TR/2010/WD-html-markup-20101019/img.html#img.attrs.border
  2. Missing alt attribute

milossh Sep 21, 2012


Both fixed in 5a4eb9e



icaaq commented Sep 21, 2012

Great! but there are more instances of with no alt and border in your commit. :)


milossh commented Sep 22, 2012

Fixed the others :)

+ var req = new XMLHttpRequest();
+ //req.open('GET', 'https://services.addons.mozilla.org/en-US/thunderbird/api/1.5/addon/lightning', false);
+ //document.domain = "kewis.ch";
+ req.open('GET', 'http://mozilla.kewis.ch/mozilla.org/lightning.xml', true);

kewisch Sep 24, 2012


Please switch this to the AMO api before making the page live


rik commented Oct 3, 2012

Sorry this review took so long. I'll have to r- for the moment.

Please open a bug to track this so that we can track this work.

The JS code should take advantage of jQuery for cross browser compatibility. I believe the CSS code is also duplicating a lot of things but I haven't taken a good look.

@sgarrity or @craigcook : Can you take a look and give recommandations?


kewisch commented Oct 3, 2012

I used foundation css for this page because it was there and easy and it also predates the time I knew this was going in through bedrock. We can easily change the CSS if bedrock provides for a way to easily layout the page and automatically make it mobile-compatible.

@milossh can you open the bug and also CC me?


sgarrity commented Feb 28, 2013

We should have reviewed this much earlier, sorry! Bedrock has changed a lot in the 5 months since this PR was filed. I think we should consider a redesign/content-updates for this page. Please file a bug to get that process started.

@sgarrity sgarrity closed this Feb 28, 2013

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