search

oppia / foundation-website

Repository for developing the new Oppia Foundation website.
Apache License 2.0
6 stars 32 forks source link

About page redesign #119

Closed hoangviet1993 closed 4 years ago

hoangviet1993 commented 4 years ago

This PR completes the redesign of the About page. Changes are live at: https://oppia-foundation-test-server.appspot.com/about

Screenshots: Desktop view (>1500px): about_redesign_desktop Tablet view (Ipad): about_redesign_tablet Mobile view (ip6/7): about_redesign_mobile

hoangviet1993 commented 4 years ago

Hi @rachelwchen Can you take a pass for the new About page?

codecov-io commented 4 years ago

Codecov Report

Merging #119 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #119   +/-   ##
=======================================
  Coverage   96.41%   96.41%           
=======================================
  Files          21       21           
  Lines         390      390           
=======================================
  Hits          376      376           
  Misses         14       14

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 519df00...bdc6231. Read the comment docs.

dchen97 commented 4 years ago

Thanks for putting this together! Just a couple of small notes:

Also, should the dropdown on the "About" button in the navbar be added in this PR or will it be a separate PR?

hoangviet1993 commented 4 years ago

For the statistic listing 1 RCT completed, can the description be re-written to "Randomized Control Trial completed (and more to come!)"

Done (here and on home page)!

In the small mobile view on Chrome, it looks like the last feature listed (Translations in local dialects) is not center-aligned:

Yup, I have now center-aligned the text in that particular div. Thanks for catching this!

At the end of the paragraph in the "Our Features" block, could you append the following: "Oppia's features include..." in order to clarify what the list that follows it is.

Done!

In "Learn more about what we do and what we've accomplishes..." - accomplishes should be accomplished.

Thanks for catching this typo! Done!

Also, should the dropdown on the "About" button in the navbar be added in this PR or will it be a separate PR?

I was thinking about doing it on a separate PR. I was considering adding that button on the "About Foundation" PR or an entirely separate PR on its own since it is a feature. Hope you understand!

@rachelwchen @dchen97 I have addressed all of your comments. Can you take a 2nd look? https://oppia-foundation-test-server.appspot.com/about

seanlip commented 4 years ago

@hoangviet1993 hm, I think you meant @dchen97 instead of @rachelwchen?

In any case, @dchen97, PTAL, thanks!

hoangviet1993 commented 4 years ago

Woopsie! That reply was indeed for Diana. Thanks Sean!

rachelwchen commented 4 years ago

Hi! Thanks so much for your work on this @hoangviet1993! Some comments:

-Spacing: in "Our Impact," the empty spacing seems odd... I wonder if it's possible to change the layout up a bit, like the screenshot below. Let me know if it's too big a fix, though... 05

-Font size: the paragraph font sizes seem inconsistent -- can we make it all the same size please? The second paragraph ("Oppia is an engaging new approach to online learning...") should be smaller, the same size as the first paragraph ("Learn more about what we do and..."). 01

-Character spacing: in the third section ("Our Impact"), the numbers on the left seem a little cramped together. Can we increase the character spacing please? image

-Button shadow: I noticed that there's a drop-shadow for all buttons on three sides -- left, bottom, and right -- however, in the "Enough of us talking about it" section, the shadow seems to only be on left and right. Can we also add it to the bottom please? 03

-Title alignment: in mobile view, all the section headers are left-aligned except for "Our Impact" (see below). Let's left-align "Our Impact" as well! 04

hoangviet1993 commented 4 years ago

Changes are live at: https://oppia-foundation-test-server.appspot.com/

-Font size: the paragraph font sizes seem inconsistent -- can we make it all the same size please? The second paragraph ("Oppia is an engaging new approach to online learning...") should be smaller, the same size as the first paragraph ("Learn more about what we do and...").

I have made the 2nd paragraph the same size as the rest as well as the bottom two cards. Does that work?

-Character spacing: in the third section ("Our Impact"), the numbers on the left seem a little cramped together. Can we increase the character spacing please?

Done!

-Button shadow: I noticed that there's a drop-shadow for all buttons on three sides -- left, bottom, and right -- however, in the "Enough of us talking about it" section, the shadow seems to only be on left and right. Can we also add it to the bottom please?

Thanks for spotting this! Done!

-Spacing: in "Our Impact," the empty spacing seems odd... I wonder if it's possible to change the layout up a bit, like the screenshot below. Let me know if it's too big a fix, though...

Can we go over this a little bit more details before I begin making changes? The original concept had both the "number and text" column and the "student testimonial" the same size. Do you want something like this?

Screen Shot 2019-07-31 at 9 41 53 PM

-Title alignment: in mobile view, all the section headers are left-aligned except for "Our Impact" (see below). Let's left-align "Our Impact" as well!

This looks like a simple change. Once we can figure out the above point, I can get this done in no time!

rachelwchen commented 4 years ago

Thank you! Re: the "Our Impact" height matching issue, yes -- if we could get it to look like the wireframe that'd be great.

hoangviet1993 commented 4 years ago

Changes in this PR are live at https://oppia-foundation-test-server.appspot.com/about

Thank you! Re: the "Our Impact" height matching issue, yes -- if we could get it to look like the wireframe that'd be great.

Done!

@rachelwchen Can you take a look again as I have addressed all of your comments. Here is a GIF to demo how the "Our Impact" look now: 2019-08-03 21 43 46

rachelwchen commented 4 years ago

@hoangviet1993 this is great!! Thanks for making the changes and making the gifs.

hoangviet1993 commented 4 years ago

@seanlip can you PTAL?