[Fix Bug 723267] Add new projects/calendar page #1466
Conversation
|
|
||
| {% block js %} | ||
| {{ js('projects-calendar') }} | ||
| <script>window.addEventListener("DOMContentLoaded", setupVersion('{{ LANG }}'), false);</script> |
There was a problem hiding this comment.
Let's remove this inline script and call setupVersion from within calendar.js. Perhaps just store the {{ LANG }} in a data attribute on the body if you need to query it.
|
Visually, the page is looking good! Only small layout issue I see is with the position of the newsletter signup button at desktop break point. Should it be to the right of the form? |
|
Since you've moved
|
| color: #FFFFFF; | ||
| line-height: 1.3; | ||
| background-color: #669BE1; | ||
| background-image: -moz-linear-gradient(center top , #669BE1 0%, #5784BF 100%); |
There was a problem hiding this comment.
We should drop the -moz prefix, or, depending on how far backwards-compatible we want to be, keep the -moz prefix and add -webkit as well as unprefixed. For this particular page I'd be happy to go with unprefixed only and older browsers can fall back to the solid color.
We also have a mixin for gradients that produces an exhaustive list if you want to be super-thorough, but it's kind of overkill. https://github.com/mozilla/bedrock/blob/master/media/css/sandstone/lib.less#L237
|
The bottom of the arrow on the button is getting clipped off. Could probably fix it with a min-height on the title and a negative bottom margin to compensate and pull the description back up (or maybe a negative top margin on the desc). The "free download" text is also a little hard to read, at least for my eyes and my screen. I think it was styled for the lighter green button and doesn't have enough contrast on the blue. Maybe go a bit darker, or switch to a light gray? Might need to adjust the text shadow also, depending on what you try. |
| background-image: -moz-linear-gradient(center top , #669BE1 0%, #5784BF 100%); | ||
| border-radius: 6px 6px 6px 6px; | ||
| box-shadow: 0 3px rgba(0, 0, 0, 0.1), 0 -4px rgba(0, 0, 0, 0.1) inset; | ||
| color: #FFFFFF; |
There was a problem hiding this comment.
Repeated color here.
And if I'm picking nits, we prefer lowercase hex and shorthand where possible, so #fff instead of #FFFFFF
|
@alexgibson Just rewrote the whole thing using jquery ajax. It also now works fine in IE10 for me and possibly earlier IE versions, though in any case, as you noted the download button fallback is pretty good so I'm not overly concerned with old IE versions on this page. Thanks for the tip about window.site.platform, and for taking the time to review! @craigcook I've fixed the following:
Thanks for the tips and your time reviewing! |
Looking good in IE10+ 👍 I believe the only way to do a cross domain request in IE <= 9 is to use the MS propriety XDomainRequest object, or else a hack like JSONP. jQuery won't support But still, like you say the link degrades perfectly well. If we aren't too concerned with these older browsers for this particular page, then that's fine with me 🎉 |
| versionBox.textContent = 'Lightning ' + $(xml).find('version').text(); | ||
| downloadBox.className = downloadBox.className.replace('loading', ''); | ||
|
|
||
| var downloadLink = downloadBox.getAttribute('href');; |
There was a problem hiding this comment.
Looks like there's still an extra semi-colon here that can be removed.
|
Ok, this one is looking good, thanks @Sancus 👍 r+ |
[Fix Bug 723267] Add new projects/calendar page
No description provided.