Replace Icomoon & font-awesome with Google Material Fonts #2093

arslanashraf7 · 2021-01-28T13:38:05Z

Pre-Flight checklist

Screenshots and design review for any changes that affect layout or styling
- Desktop screenshots
- Mobile width screenshots
Testing
- Code is tested
- Changes have been manually tested

What are the relevant tickets?

#2065

What's this PR do?

Replaces the usage of Icomoon & font-awesome with Google Material Icons.

How should this be manually tested?

Need to verify the icons and their usage is intact.
Need to verify that there is not a lot of difference between the look & feel of the Icons used through Google Material Font

Any background context you want to provide?

In past we were using icons from 3 different sources Google Material, Icomoon & Font-Awesome. Now, While we are in the process of improving the performance of the website so we decided to use the icons just from Google Material Fonts which will provide use some performance improvement since we'll have to load just Google Font reducing the page load time taken by Icomoon & Font-Awesome.

Screenshots (if appropriate)

Notification Cancel Icon
Old

New

Course Carousel
Old

New

Testimonials Carousel
Old

New

Catalog Details
Old

New

Testimonials Dialog Cancel
Old

New

Back to top
Old

New

Mobile Screenshots

Notification's Cancel Button
Old

New

Course Carousel
Old

New

Testimonials Carousel
Old

New

Catalog Details
Old

New

Testimonial Dialog Cancel
Old

New

Back to Top Button
Old

New

codecov-io · 2021-01-28T13:48:16Z

Codecov Report

Merging #2093 (8dc1c5b) into master (82d57b0) will decrease coverage by 4.58%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2093      +/-   ##
==========================================
- Coverage   88.00%   83.41%   -4.59%     
==========================================
  Files         328      171     -157     
  Lines       14767     9833    -4934     
  Branches     1031     1031              
==========================================
- Hits        12995     8202    -4793     
+ Misses       1533     1392     -141     
  Partials      239      239

Impacted Files	Coverage Δ
static/js/components/input/ProductSelector.js
static/js/lib/urls_test.js
static/js/reducers/notifications_test.js
static/js/factories/course.js
static/js/components/Markdown.js
static/js/reducers/notifications.js
...s/containers/pages/profile/EditProfilePage_test.js
static/js/containers/pages/login/LoginPages.js
static/js/containers/pages/profile/ProfilePages.js
...ainers/pages/login/LoginForgotPasswordPage_test.js
... and 146 more

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 82d57b0...8dc1c5b. Read the comment docs.

arslanashraf7 · 2021-01-29T13:39:06Z

@briangrossman & @abdulkdawson , We might need you to review this PR where we are basically replacing all the Font-Awesome and Icomoon Icons with Google Material Fonts.

I've mentioned the reason for doing it in the PR description's Background Context part.

Please have a look and let me know if you approve of these changes.

briangrossman · 2021-02-01T20:01:23Z

@arslanashraf7 Thank you for the detailed list of changes. All of the changes you show above look good to me.

Looks like we're switching from filled-in arrows and not-filled-in close buttons to not-filled-in arrows and filled-in close buttons. :) That's fine.

And the replacement >'s look fine, too.

This is all good from a look and feel perspective.

arslanashraf7 · 2021-02-02T11:22:15Z

@arslanashraf7 Thank you for the detailed list of changes. All of the changes you show above look good to me.

Looks like we're switching from filled-in arrows and not-filled-in close buttons to not-filled-in arrows and filled-in close buttons. :) That's fine.

And the replacement >'s look fine, too.

This is all good from a look and feel perspective.

@briangrossman Thanks for reviewing the designs, Also your perceptions are right on moving towards basic outline icons. Now, I'll just wait on @gsidebo's review to take this change into master.

gsidebo

This looks great! Just the one comment

gsidebo · 2021-02-04T21:19:32Z

static/js/components/TopAppBar.js

@@ -22,7 +22,7 @@ const shouldShowLoginSignup = location =>
  )

 const TopAppBar = ({ currentUser, location }: Props) => (
-  <header className="header-holder">
+  <header className="header-holder" style={{ height: 98 }}>


The purpose of setting a height on the header is to have an element of the correct height occupying the space before the header-related Javascript is loaded. Setting the height here doesn't help with that. You need to add this height to an element in the HTML template

You're right, I've updated it, I've moved this to HTML templates where TopAppBar was used and where it was possible to set fixed height without effecting responsiveness. It definitely saved us some layout shifts now.

gsidebo · 2021-02-09T16:56:56Z

cms/templates/catalog_page.html

@@ -5,7 +5,7 @@
 {% block title %}Catalog | {{ site_name }}{% endblock %}

 {% block headercontent %}
-  <div id="header"></div>
+  <div id="header" style="height: 98px"></div>


I'm pretty sure we can just add this to the #header selector in static/scss/detail/header.scss instead of inlining it in all of these files

Looks like a great suggestion, I'll change/test it in CSS and will update the PR shortly.

gsidebo

Looks great! 👍

arslanashraf7 · 2021-02-09T17:41:40Z

Looks great! 👍

@gsidebo Thanks for the quick review.

arslanashraf7 added the Work in Progress label Jan 28, 2021

odlbot temporarily deployed to xpro-ci-pr-2093 January 28, 2021 13:38 Inactive

odlbot temporarily deployed to xpro-ci-pr-2093 January 28, 2021 14:48 Inactive

odlbot temporarily deployed to xpro-ci-pr-2093 January 28, 2021 15:17 Inactive

odlbot temporarily deployed to xpro-ci-pr-2093 January 29, 2021 07:02 Inactive

odlbot temporarily deployed to xpro-ci-pr-2093 January 29, 2021 07:32 Inactive

odlbot temporarily deployed to xpro-ci-pr-2093 January 29, 2021 10:00 Inactive

odlbot temporarily deployed to xpro-ci-pr-2093 January 29, 2021 10:34 Inactive

odlbot temporarily deployed to xpro-ci-pr-2093 January 29, 2021 10:45 Inactive

arslanashraf7 force-pushed the arslan/use-material-font branch from cea8645 to c1e467b Compare January 29, 2021 10:46

odlbot temporarily deployed to xpro-ci-pr-2093 January 29, 2021 10:46 Inactive

odlbot temporarily deployed to xpro-ci-pr-2093 January 29, 2021 12:19 Inactive

arslanashraf7 force-pushed the arslan/use-material-font branch from 4e4729a to 115e4d4 Compare January 29, 2021 12:19

odlbot temporarily deployed to xpro-ci-pr-2093 January 29, 2021 12:20 Inactive

odlbot temporarily deployed to xpro-ci-pr-2093 January 29, 2021 12:28 Inactive

odlbot deployed to xpro-ci-pr-2093 January 29, 2021 13:12 View deployment

arslanashraf7 requested review from gsidebo, abdulkdawson and briangrossman January 29, 2021 13:36

arslanashraf7 added Needs Review and removed Work in Progress labels Jan 29, 2021

arslanashraf7 mentioned this pull request Jan 29, 2021

EPIC: xPRO performance improvements #2065

Closed

3 tasks

gsidebo self-assigned this Feb 2, 2021

gsidebo suggested changes Feb 4, 2021

View reviewed changes

gsidebo added Waiting on Author and removed Needs Review labels Feb 4, 2021

arslanashraf7 force-pushed the arslan/use-material-font branch from a206e59 to 52f8815 Compare February 9, 2021 12:12

arslanashraf7 requested a review from gsidebo February 9, 2021 12:33

arslanashraf7 added Needs Review and removed Waiting on Author labels Feb 9, 2021

gsidebo suggested changes Feb 9, 2021

View reviewed changes

gsidebo added Waiting on Author and removed Needs Review labels Feb 9, 2021

arslanashraf7 force-pushed the arslan/use-material-font branch from 52f8815 to b588f3b Compare February 9, 2021 17:26

gsidebo approved these changes Feb 9, 2021

View reviewed changes

gsidebo added Needs Review and removed Waiting on Author labels Feb 9, 2021

arslanashraf7 added Waiting on Author and removed Needs Review labels Feb 9, 2021

arslanashraf7 force-pushed the arslan/use-material-font branch from b588f3b to 94eadc3 Compare February 9, 2021 17:43

Replace Font-Awesome & Icomoon with Google Font

c97afab

arslanashraf7 force-pushed the arslan/use-material-font branch from 94eadc3 to c97afab Compare February 9, 2021 17:44

arslanashraf7 merged commit 8e2f561 into master Feb 9, 2021

arslanashraf7 deleted the arslan/use-material-font branch February 9, 2021 18:00

HamzaIbnFarooq mentioned this pull request Feb 10, 2021

Add news and events carousel #2111

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace Icomoon & font-awesome with Google Material Fonts #2093

Replace Icomoon & font-awesome with Google Material Fonts #2093

arslanashraf7 commented Jan 28, 2021 •

edited

Loading

codecov-io commented Jan 28, 2021 •

edited

Loading

arslanashraf7 commented Jan 29, 2021

briangrossman commented Feb 1, 2021

arslanashraf7 commented Feb 2, 2021

gsidebo left a comment

gsidebo Feb 4, 2021

arslanashraf7 Feb 9, 2021

gsidebo Feb 9, 2021

arslanashraf7 Feb 9, 2021

gsidebo left a comment

arslanashraf7 commented Feb 9, 2021

Replace Icomoon & font-awesome with Google Material Fonts #2093

Replace Icomoon & font-awesome with Google Material Fonts #2093

Conversation

arslanashraf7 commented Jan 28, 2021 • edited Loading

Pre-Flight checklist

What are the relevant tickets?

What's this PR do?

How should this be manually tested?

Any background context you want to provide?

Screenshots (if appropriate)

codecov-io commented Jan 28, 2021 • edited Loading

Codecov Report

arslanashraf7 commented Jan 29, 2021

briangrossman commented Feb 1, 2021

arslanashraf7 commented Feb 2, 2021

gsidebo left a comment

Choose a reason for hiding this comment

gsidebo Feb 4, 2021

Choose a reason for hiding this comment

arslanashraf7 Feb 9, 2021

Choose a reason for hiding this comment

gsidebo Feb 9, 2021

Choose a reason for hiding this comment

arslanashraf7 Feb 9, 2021

Choose a reason for hiding this comment

gsidebo left a comment

Choose a reason for hiding this comment

arslanashraf7 commented Feb 9, 2021

arslanashraf7 commented Jan 28, 2021 •

edited

Loading

codecov-io commented Jan 28, 2021 •

edited

Loading