(WIP) Fixes #856 new homepage #943

mmmavis · 2015-05-26T05:52:45Z

This fixes #856

[ WIP ]

Missing proper tests
should render longer blog post excerpt
3 buttons on hero unit
optimize images (see (WIP) Fixes #856 new homepage #943 (comment))
update changelog

toolness · 2015-05-26T14:48:55Z

less/index.less

@@ -4,3 +4,4 @@
 @import "components/index";
 @import "pages/index";
 @import "../node_modules/react-select/less/default.less";
+@import "../node_modules/react-select/less/default.less";


Hmm why is the same file imported twice?

purely by mistake 😁

toolness · 2015-05-26T15:07:03Z

The new hero-unit.png and its 2x variant are 160k and 560k, respectively, which is quite large. Can you optimize the file sizes a bit?

toolness · 2015-05-26T15:09:42Z

pages/clubs.jsx

              imgSrc="/img/pages/clubs/mikko-finland.png" imgSrc2x="/img/pages/clubs/[email protected]" imgAlt="Mikko Finland Quote">

-            <p>The idea of teachers and students learning at the same time is what makes me excited about this work.</p>
+            <p>"The idea of teachers and students learning at the same time is what makes me excited about this work."</p>


I think it'd be swell to use “ and ” for typographic awesomeness here but it's up to you 😁

Maybe using CSS pseudo-elements :before and :after could help? And I guess we don't have to pass imgAlt here as well since the image is decorative

OR...
I wonder if we could use <q> to wrap around the actual quote instead of <p>. I tested it with VoiceOver and it interpreted the two(<q> vs <p>) the same.

Oh neat, I didn't even know <q> was a tag!

toolness · 2015-05-26T15:18:50Z

Err, sorry I started reviewing this before you were finished--just figured I'd get a head start, but please let me know if my comments were unhelpful.

mmmavis · 2015-05-26T18:36:44Z

not a problem as all, and thanks for those suggestions and comments

mmmavis · 2015-05-26T18:42:14Z

it's better to know early whether or not i'm on the right track! 💃

mmmavis · 2015-05-27T00:35:32Z

pages/home.jsx

+      <div className="featured-post">
+        <div className="entry-posted-container">
+          <p className="entry-posted">
+            <time className="published" title={this.props.data.publishedDate} dateTime={this.props.data.publishedDate} >


leaving dateTime value unparsed here since it's in machine readable format already

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/time

This element is intended to be used presenting dates and times in a machine readable format. This can be helpful for user agents to offer any event scheduling for user's calendar.

toolness · 2015-05-27T20:16:17Z

less/common.less

+#content {
+  padding-left: 0;
+  padding-right: 0;
+}


Hmm, unfortunately this seems to break other pages. For instance, in /about/, the heading "About Mozilla Learning Networks" bleeds all the way to the very edge of the screen on mobile, whereas there's normally 15px of padding being applied from bootstrap's .col-* rules. 😢

arghhh, if #content does have left/right padding we won't be able to have a full width section with background colour (e.g., the quote section on the new homepage). I probably have to go through all the pages and make sure things are being wrapped within extra inner containers... 😭

Oh, can you set negative margin to offset the padding? I actually did that with the hero unit for the original site, way back in February: https://github.com/mozilla/teach.webmaker.org/blob/8804e410ce96df5d2983fe829b6a81c335e94d74/styles.less#L23-L26

I'm not sure if that CSS has actually survived to present-day or not, hehe.

Argh, I tried the following on that full width coloured section

margin-left: -(@grid-gutter-width / 2); padding-left: (@grid-gutter-width / 2);

This leaves a gap to the right due to the negative offset on the left.

Adding margin-right: -(@grid-gutter-width / 2); simply just makes the section wider but the gap is still there, which introduces a horizontal scrollbar to the page.

Also, this approach won't work on larger screen either as we can't specify how much offset we want to move. (see https://github.com/mmmavis/teach.webmaker.org/blob/develop/less/components/page.less#L21-L23)

😭

toolness · 2015-05-27T21:11:43Z

Hmm, I guess don't actually worry about fixing any of the feed API stuff now. I should've seen this complexity coming and had us split this up into two separate PRs, one for the HTML/CSS of the page and the other for the feed stuff. It's way too much work for one person!

toolness · 2015-05-27T21:12:14Z

I just noticed that the rightmost icon link actually looks a bit wonky when the page is narrow:

toolness · 2015-05-27T22:05:25Z

Ok I just filed mmmavis#3! If you accept that, then we can work in parallel and eventually UNITE.

toolness · 2015-05-27T22:11:41Z

By the way, when I load your PR in Chrome, the following is logged to my browser console when viewing the fancy new homepage:

Deprecation warning: moment construction falls back to js Date. This is discouraged and will be removed in upcoming major release. Please refer to moment/moment#1407 for more info.

Add a stub/fake blog feed loader and use it for now.

… solution This reverts commit 57b7799.

mmmavis · 2015-05-28T01:06:58Z

oh darn I don't get why
the text didn't get wrappppppppped. Gonna come back to this issue tonight

mmmavis · 2015-05-28T01:12:23Z

Hey @toolness ,

As I investigated the margin issue I went through all the pages and wrapped sections within <section>. We have done that on some pages but it's just we haven't been consistent about it. If you wanna view the code changes you can add ?w=1 to the URL to hide all the whitespace differences in the PR. e.g., https://github.com/mozilla/teach.webmaker.org/pull/943/files?w=1

mmmavis · 2015-05-28T03:34:32Z

less/components/btn.less

@@ -1,4 +1,5 @@
 .btn {
+  white-space: normal;


button text should be wrapped. we shouldn't assume button text can always fit into one line, especially the site will be localized in the future.

That's a good point, do you think you can actually add that as a comment in the code?

mmmavis · 2015-05-28T10:15:50Z

@toolness unit test PR is here. Still WIP though but I had some questions for you there 💁

toolness · 2015-05-28T10:49:07Z

pages/about.jsx

-          </section>
-        </div>
+      <section className="intro">
+        <h1>About Mozilla Learning Networks</h1>


Oof, changes like this made this PR harder to understand, with all the other changes going on in it 😭

If it's still possible, do you think you can actually undo these kinds of changes that are purely indentation changes due to being wrapped in a new element? We could then redo the indentation fixes in a separate PR after the homepage lands... if it's too much trouble though, don't worry about it.

Oh wait nevermind! I didn't see your note about ?w=1, that's 💥 AWESOME 💥

…orking; introduced a cleaner solution

mmmavis · 2015-05-28T23:29:12Z

I opened a cleaner version of this PR and it got merged. Gonna close this one now.

mmmavis added the PR needs work label May 26, 2015

toolness reviewed May 26, 2015
View reviewed changes

mmmavis added 5 commits May 26, 2015 14:53

(WIP) new homepage. missing blog section

96cda43

added blog section with hardcoded content

a9c909a

hooked up with google feed api, removed hardcoded data

271e8fa

small code cleanup and tweaks

9c4ce6e

Added moment.js & updated Twitter link

280c772

mmmavis force-pushed the issue-856-new-homepage branch from c81060c to 280c772 Compare May 27, 2015 00:29

mmmavis reviewed May 27, 2015
View reviewed changes

mmmavis added 2 commits May 26, 2015 18:04

ui tweaks

bf01bcb

blog post excerpt, hero unit buttons

08a6199

mmmavis mentioned this pull request May 27, 2015

Optimize hero unit images #946

Closed

mmmavis added 3 commits May 26, 2015 23:57

fixed error thrown by tests

ba352d3

forgot to include a module in package.json

b8988aa

code cleanup & update CHANAGE LOG

c55112e

toolness reviewed May 27, 2015
View reviewed changes

Add stub-blog-feed-loader.js and use it for now.

5152a87

toolness mentioned this pull request May 27, 2015

Add a stub/fake blog feed loader and use it for now. mmmavis/learning.mozilla.org#3

Merged

toolness and others added 5 commits May 27, 2015 18:48

remove 'Data' suffix from state keys too.

86fb847

Merge pull request #3 from toolness/decouple-new-homepage-from-googs

7f591c6

Add a stub/fake blog feed loader and use it for now.

some code cleanup & wrapped sections in <main> with <section> tags

26068a0

undo some css rules

57b7799

Revert "undo some css rules" - this fix doesn't work on larger screen…

ea31f73

… solution This reverts commit 57b7799.

mmmavis added 2 commits May 27, 2015 18:47

fixed moment date parsing warning

991d62d

styling tweaks - long button text should be wrapped

5f450ff

mmmavis reviewed May 28, 2015
View reviewed changes

styling tweaks

fb796d3

toolness reviewed May 28, 2015
View reviewed changes

toolness mentioned this pull request May 28, 2015

Add google-blog-feed-loader w/ automated and manual tests #950

Merged

undo early css changes made to make the full width coloured section w…

95c4622

…orking; introduced a cleaner solution

mmmavis mentioned this pull request May 28, 2015

CLEANER PR for #856 the new homepage implementation #952

Merged

mmmavis closed this May 28, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(WIP) Fixes #856 new homepage #943

(WIP) Fixes #856 new homepage #943

mmmavis commented May 26, 2015

toolness May 26, 2015

mmmavis May 26, 2015

toolness commented May 26, 2015

toolness May 26, 2015

mmmavis May 26, 2015

mmmavis May 26, 2015

toolness May 26, 2015

toolness commented May 26, 2015

mmmavis commented May 26, 2015

mmmavis commented May 26, 2015

mmmavis May 27, 2015

toolness May 27, 2015

mmmavis May 27, 2015

toolness May 27, 2015

mmmavis May 28, 2015

toolness commented May 27, 2015

toolness commented May 27, 2015

toolness commented May 27, 2015

toolness commented May 27, 2015

mmmavis commented May 28, 2015

mmmavis commented May 28, 2015

mmmavis May 28, 2015

toolness May 28, 2015

mmmavis commented May 28, 2015

toolness May 28, 2015

toolness May 28, 2015

mmmavis commented May 28, 2015

(WIP) Fixes #856 new homepage #943

(WIP) Fixes #856 new homepage #943

Conversation

mmmavis commented May 26, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

toolness commented May 26, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

toolness commented May 26, 2015

mmmavis commented May 26, 2015

mmmavis commented May 26, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

toolness commented May 27, 2015

toolness commented May 27, 2015

toolness commented May 27, 2015

toolness commented May 27, 2015

mmmavis commented May 28, 2015

mmmavis commented May 28, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmmavis commented May 28, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmmavis commented May 28, 2015