Fixed #5425: Redesign fractions landing page #5527

unit-00 · 2018-08-15T05:17:50Z

Explanation

Fixes #5425
This is a PR for the fractions landing page redesign according to the mock up.

The desktop version looks similar to the mock up and below is a sample of the mobile version.

Checklist

The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes.
The PR explanation includes the words "Fixes #bugnum: ...".
The linter/Karma presubmit checks have passed.
- These should run automatically, but if not, you can manually trigger them locally using python scripts/pre_commit_linter.py and bash scripts/run_frontend_tests.sh.
The PR is made from a branch that's not called "develop".
The PR follows the style guide.
The PR is assigned to an appropriate reviewer.
- If you're a new contributor, please ask on Gitter for someone to assign a reviewer.
- If you're not sure who the appropriate reviewer is, please assign to the issue's "owner" -- see the "talk-to" label on the issue.

…explore lessons button

codecov-io · 2018-08-15T06:12:18Z

Codecov Report

Merging #5527 into develop will decrease coverage by <.01%.
The diff coverage is 42.85%.

@@             Coverage Diff             @@
##           develop    #5527      +/-   ##
===========================================
- Coverage    45.83%   45.83%   -0.01%     
===========================================
  Files          511      511              
  Lines        29819    29826       +7     
  Branches      4514     4514              
===========================================
+ Hits         13669    13672       +3     
- Misses       16150    16154       +4

Impacted Files	Coverage Δ
...ates/dev/head/pages/landing/fractions/Fractions.js	`6.25% <0%> (-2.09%)`	⬇️
...v/head/domain/utilities/UrlInterpolationService.js	`93.33% <100%> (+0.27%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0ec3d94...ff59c27. Read the comment docs.

seanlip · 2018-08-16T09:19:10Z

Thanks @unit-00! Let me start with UI notes first, based on your very helpful screen animation.

Please read the original email carefully and make sure that all amendments to the Figma mocks are addressed. In particular, the "pizza"-looking cake should be removed from the first graphic.
As you scroll down, the card in the second section seems to disappear?
The bottom "Get Started" button does not seem centralized properly.

Thanks!

seanlip · 2018-08-16T09:20:43Z

core/templates/dev/head/pages/landing/fractions/Fractions.js

@@ -23,6 +23,8 @@ oppia.controller('Fractions', [
      siteAnalyticsService, UrlInterpolationService) {
    $scope.getStaticImageUrl = UrlInterpolationService.getStaticImageUrl;

+    $scope.getStaticAssetUrl = UrlInterpolationService.getStaticAssetUrl;


For parity with image, let's define getStaticVideoUrl similarly in UrlInterpolationService and add unit tests for it.

Will definitely add this, along with the unit test.

seanlip · 2018-08-16T09:22:44Z

(I only had one comment. On the whole, this looks pretty good. Thanks @unit-00!)

unit-00 · 2018-08-16T22:23:53Z

Hi @seanlip ,

I completely misunderstood what you meant on the pizza. I'll definitely remove it.
Thank you for catching this. I'm unsure of the reason why the card is disappearing, I'll look into it.
I'll be sure to fix this up.

…ticVideoUrl

unit-00 · 2018-08-20T00:12:22Z

Hey @seanlip ,

I've removed the pizza, centered the buttons, added getStaticVideoUrl and its respective tests, but I think the white part disappearing might be a gif issue.

Everything else seems to be working the way that it is on both desktop and mobile.

seanlip · 2018-08-20T06:30:43Z

core/templates/dev/head/pages/landing/fractions/landing_page_teacher.html

        <div class="container-fluid">
          <div class="row">
            <div class="col-sm-6 col-sm-push-6 oppia-fractions-landing-image-div" style="height: auto">
-              <img ng-src="<[getStaticImageUrl('/fractions_landing/student/intro_matthew.svg')]>" class="oppia-fractions-landing-image" style="width: 60%" alt="">
+              <img ng-src="<[getStaticImageUrl('/fractions_landing/teachers/matthew_pizza.png')]>" class="oppia-fractions-landing-image" style="width: 60%" alt="">


Can we name this image something that doesn't have "pizza" in it?

seanlip · 2018-08-20T06:38:23Z

Hi @unit-00, thanks! Just found a couple of additional UI issues when testing:

The video isn't loading for me. I think the case of "fractions" is wrong in the video filename?
On narrow screens, sometimes the text falls off the screen -- see "Easy-to-follow lessons" in the first screenshot below.
There are some CSS issues with the text on the buttons ("Get Started", "Learn More") being much too small on narrow screens. See both screenshots below -- you need to squint to see what's on the buttons:

$fractions-test$

$fractions-test-2$

Would it be possible to fix these? I think it should be fully ready after that.

Thanks!

ports."

unit-00 · 2018-08-29T03:55:41Z

Hi @seanlip ,

I believe I've address the issues along with making some additional change to the overall look of the page.

Please look them over!

apb7 · 2018-08-29T05:59:43Z

scripts/pre_commit_linter.py

@@ -221,7 +221,7 @@
    'third_party/*', 'build/*', '.git/*', '*.pyc', 'CHANGELOG',
    'integrations/*', 'integrations_dev/*', '*.svg', '*.gif',
    '*.png', '*.zip', '*.ico', '*.jpg', '*.min.js',
-    'assets/scripts/*', 'core/tests/data/*', '*.mp3')
+    'assets/scripts/*', 'core/tests/data/*', '*.mp3', '*.mp4')


LGTM for the changes made in this file only. Thanks!

seanlip · 2018-08-29T09:26:14Z

Thanks @unit-00, it's looking good! Just one comment:

Seems like the bottom button is not centered properly. (And the top one too, for that matter, comparing with the text just above it?)

Also, the text on the buttons is now a little too big. Could we make it a little smaller? It should probably be just a bit bigger than the "Get your students ... lessons" text. Ditto for the buttons at the bottom of the page.

Finally, could we embed the video in a phone outline, something like this? https://pixabay.com/en/smartphone-icon-modern-symbol-1557796/

Thanks!

unit-00 · 2018-08-29T20:05:03Z

Hey @seanlip ,

quick feedback. How does this look from the text size above to the button's text size.

Similarly,

seanlip · 2018-08-29T20:15:15Z

Looks good to me, thanks!

…text above, add a new media query, put text description in a container

seanlip

Oh wow, I just realized I hadn't submitted this review yet, sorry! Doing it now.

Also, any chance we could get this in soon? Want to include it in the September release...

Thanks!

seanlip · 2018-08-29T09:20:46Z

core/templates/dev/head/pages/landing/fractions/landing_page_teacher.html

-              <button class="btn oppia-fractions-landing-get-started" ng-click="onClickGetStartedButton('teacher')">Get Started</button>
+            <div class="col-sm-6 col-sm-pull-6" style="z-index:20">
+              <h1 class="oppia-fractions-landing-h1">Fractions just got easier</h1>
+              <h2 class="oppia-fractions-landing-h2" style="padding-right:15px;">Get your students and kids started with our free, effective math lessons.</h2>


Nit: add space after ":" in the style attr.

seanlip · 2018-08-29T09:20:49Z

core/templates/dev/head/pages/landing/fractions/landing_page_teacher.html

+            <div class="col-sm-6 col-sm-pull-6" style="z-index:20">
+              <h1 class="oppia-fractions-landing-h1">Fractions just got easier</h1>
+              <h2 class="oppia-fractions-landing-h2" style="padding-right:15px;">Get your students and kids started with our free, effective math lessons.</h2>
+              <button class="btn oppia-fractions-landing-get-started" ng-click="onClickGetStartedButton('teacher')" style="left:10px;">Get Started</button>


Nit: add space after ":" in the style attr.

unit-00 · 2018-09-06T22:07:27Z

Hey @seanlip ,

I great apologize for the delay. I got a bit too frustrated with making sure the video align properly in the smartphone image.

I wasn't sure how to exactly arrange the video inside the image so I made a border to look like a smartphone instead.

seanlip

Hi @unit-00, just took a look, and I think it looks amazing! I love what you did with the phone, it looks great. Sorry to hear that it was a bit frustrating though.

I left a few code comments! UI comments coming shortly.

seanlip · 2018-09-07T10:26:15Z

core/templates/dev/head/pages/landing/fractions/landing_page_teacher.html

      max-height: 500px;
    }
    .oppia-fractions-landing-image-mobile {
      display: none;
    }
+    .video-frame {


Could you please prefix this class name with .oppia-fractions-landing-... ? Ditto for anything below this one as well (text-box, easy-to-follow, etc.) -- want to make sure the CSS is well-scoped and only applies to this page.

Oh ops, didn't see these reviews. I'll definitely fix these.

seanlip · 2018-09-07T10:26:35Z

core/templates/dev/head/pages/landing/fractions/landing_page_teacher.html

      max-height: 500px;
    }
    .oppia-fractions-landing-image-mobile {
      display: none;
    }
+    .video-frame {
+      position: relative;


For each CSS rule please alphabetize (e.g. background then border then border-bottom-width etc.) Ditto below.

seanlip · 2018-09-07T10:27:09Z

core/templates/dev/head/pages/landing/fractions/landing_page_teacher.html

      }
      .oppia-fractions-landing-h2 {
        font-size: 1em;
-        text-align: left !important;
+        text-align: center !important;


Is it possible at all to get the desired effect without using !important, here and above? In general we try to avoid it.

This was actually from whoever first designed this. Should I take out all the !important? I edited it a bit to change certain stylings.

seanlip · 2018-09-07T10:33:34Z

A couple of minor UI things:

When you hover over the video in such a way that clicking would play it, the mouse pointer should look like something that can click (I think a hand, usually?).
On larger widths, the text between the buttons at the bottom is not centered -- in the screenshot below, you'll see that the text juts out to the right more than the left:

Also, could we change the last exclamation mark (after "Library") to a period? Two exclamation marks in one go seems a bit much IMO.

Otherwise, it looks really nice, thanks a lot! Just so you know, the release cut for September has been delayed to this weekend, so if you could finish this and submit it by tomorrow or early Saturday, then it can go out in the September release.

Thanks!

…last section

unit-00 · 2018-09-07T21:22:54Z

Hi @seanlip ,

I've centered the the library text and also change the exclamation mark to the period.

However it might be a bit harder to change the cursor for the html5 video.
Based on these SO posts, SO 0 and SO 1, it seems as though the controls are set by the browser.

~~I've been trying but it's only allowing me to change the cursor for the timeline, the volume, and the fullscreen button, not the play/pause, I'm going to see if I can do it some other way.~~

Edit: I was able to change everything to pointers upon hover and got around the play button issue. I've also took out the !important css stylings.

Please look over the code and see if I've address everything.

Thank you.

…rtant in css styling and adjusting styling in response to removing important

on it instead of just play button

seanlip · 2018-09-08T09:19:29Z

Hi @unit-00, this looks awesome, thanks so much! Merging now :) -- it'll be live in mid-Sept once the release has been QA'd and pushed!

unit-00 added 11 commits August 4, 2018 18:44

Change descriptions, change coloring, and also change font size, add …

95a8d0f

…explore lessons button

Move h1 titles into flexbox and shifted left to align with h2 titles

ff1a2b2

Change images to mock and also reduce space between the title and text

c850589

Add image files for fraction page

1542682

Merge remote-tracking branch 'upstream/develop' into issue-5425

55662e6

Merge remote-tracking branch 'upstream/develop' into issue-5425

5577147

Redesign fractions landing page, add video directory to asset

f0a57d3

Remov autoplay from videos and remove placeholder image

97135da

Add video type back

312c7ef

Add mp4 to prelint checker

d3f59eb

Merge remote-tracking branch 'upstream/develop' into issue-5425

ce3e994

unit-00 requested a review from seanlip August 15, 2018 05:18

seanlip reviewed Aug 16, 2018

View reviewed changes

unit-00 mentioned this pull request Aug 19, 2018

Implement a redesign version of page at www.oppia.org/fractions #5425

Closed

4 tasks

unit-00 added 4 commits August 19, 2018 15:36

Merge remote-tracking branch 'upstream/develop' into issue-5425

0a0048c

Remove pizza, center buttons on last section, and add test for getSta…

87b019d

…ticVideoUrl

Fix unclose bracket in UrlInterpolationService.js

df73b52

Change getStaticAssetUrl to getStaticVideoUrl

746964b

seanlip reviewed Aug 20, 2018

View reviewed changes

unit-00 added 5 commits August 20, 2018 15:12

rename Fractions_video.mp4 in cache to fractions_video.mp4

3fb47df

rename matthew_pizza.png to matthew_paper.png

8a2f06d

"Address review issues: change button and title size for different view

655b344

ports."

"Increase button size even more to be easier on the eyes"

c6eaa48

Merge remote-tracking branch 'upstream/develop' into issue-5425

336e43a

unit-00 added 2 commits August 28, 2018 19:47

Merge remote-tracking branch 'upstream/develop' into issue-5425

97fe009

Align get-started and learn-more buttons with the text above

2a8f402

unit-00 requested a review from apb7 as a code owner August 29, 2018 03:46

apb7 reviewed Aug 29, 2018

View reviewed changes

remove left shifting, change button text size to be proportionate to …

56ac98f

…text above, add a new media query, put text description in a container

seanlip reviewed Sep 3, 2018

View reviewed changes

unit-00 added 2 commits September 6, 2018 15:02

add smartphone border

b45121f

Merge remote-tracking branch 'upstream/develop' into issue-5425

2cfe109

fill in gap between video and border

bdbe13b

seanlip requested changes Sep 7, 2018

View reviewed changes

unit-00 added 2 commits September 7, 2018 14:17

center library text and align get started and explore buttons in the …

76a9bd8

…last section

Merge remote-tracking branch 'upstream/develop' into issue-5425

0805461

unit-00 added 5 commits September 7, 2018 14:32

add oppia-fractions-landing prefix and alphabetize css rules

bc01149

remove easy-to-follow class to add styling to overall h1, remove impo…

41104e8

…rtant in css styling and adjusting styling in response to removing important

"made font size smaller a bit"

e1f8084

Change cursor to pointer on video hover, and video plays when clicking

02306cb

on it instead of just play button

Change "video-frame" class to "oppia-fractions-landing-video-frame"

ff59c27

seanlip approved these changes Sep 8, 2018

View reviewed changes

seanlip merged commit f112523 into oppia:develop Sep 8, 2018

unit-00 deleted the issue-5425 branch September 8, 2018 14:12

This was referenced Sep 16, 2018

Update text on home and some small design changes for the Home page. oppia/foundation-website#63

Merged

Hovering over the Play button in the homepage video should cause the mouse cursor to turn into a pointer. oppia/foundation-website#66

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed #5425: Redesign fractions landing page #5527

Fixed #5425: Redesign fractions landing page #5527

unit-00 commented Aug 15, 2018

codecov-io commented Aug 15, 2018 •

edited

seanlip commented Aug 16, 2018

seanlip Aug 16, 2018

unit-00 Aug 16, 2018

seanlip commented Aug 16, 2018

unit-00 commented Aug 16, 2018

unit-00 commented Aug 20, 2018

seanlip Aug 20, 2018

seanlip commented Aug 20, 2018

unit-00 commented Aug 29, 2018

apb7 Aug 29, 2018

seanlip commented Aug 29, 2018

unit-00 commented Aug 29, 2018

seanlip commented Aug 29, 2018 via email

seanlip left a comment

seanlip Aug 29, 2018

seanlip Aug 29, 2018

unit-00 commented Sep 6, 2018 •

edited

seanlip left a comment

seanlip Sep 7, 2018

unit-00 Sep 7, 2018

seanlip Sep 7, 2018

unit-00 Sep 7, 2018

seanlip Sep 7, 2018

unit-00 Sep 7, 2018 •

edited

seanlip commented Sep 7, 2018

unit-00 commented Sep 7, 2018 •

edited

seanlip commented Sep 8, 2018

Fixed #5425: Redesign fractions landing page #5527

Fixed #5425: Redesign fractions landing page #5527

Conversation

unit-00 commented Aug 15, 2018

Explanation

Checklist

codecov-io commented Aug 15, 2018 • edited

Codecov Report

seanlip commented Aug 16, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seanlip commented Aug 16, 2018

unit-00 commented Aug 16, 2018

unit-00 commented Aug 20, 2018

Choose a reason for hiding this comment

seanlip commented Aug 20, 2018

unit-00 commented Aug 29, 2018

Choose a reason for hiding this comment

seanlip commented Aug 29, 2018

unit-00 commented Aug 29, 2018

seanlip commented Aug 29, 2018 via email

seanlip left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

unit-00 commented Sep 6, 2018 • edited

seanlip left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

unit-00 Sep 7, 2018 • edited

Choose a reason for hiding this comment

seanlip commented Sep 7, 2018

unit-00 commented Sep 7, 2018 • edited

seanlip commented Sep 8, 2018

codecov-io commented Aug 15, 2018 •

edited

unit-00 commented Sep 6, 2018 •

edited

unit-00 Sep 7, 2018 •

edited

unit-00 commented Sep 7, 2018 •

edited