feat(teams page): Design a teams page #202

fredrik-lundin · 2017-12-13T20:27:14Z

Initial design and implementation of a Teams page #14

ReactiveX#14

fredrik-lundin · 2017-12-13T20:30:53Z

@ladyleet this is my first ever PR to an open source repo, so please help me out if I'm doing something wrong!

btroncone · 2017-12-13T20:31:44Z

Awesome, welcome @fredrik-lundin!

sumitarora · 2017-12-13T20:31:59Z

Thanks @fredrik-lundin 😄

should have been removed in previous merge

sumitarora · 2017-12-13T20:34:17Z

yarn-error.log

@@ -0,0 +1,7574 @@
+Arguments: 
+  C:\Program Files\nodejs\node.exe C:\Users\fredrikl\AppData\Roaming\npm\node_modules\yarn\bin\yarn.js


Please delete this file.

sumitarora · 2017-12-13T20:36:55Z

.angular-cli.json

      "styles": [
-        "styles.scss"
+        "styles.scss",
+        "../node_modules/font-awesome/scss/font-awesome.scss"


Please use import in styles.scss like https://github.com/ReactiveX/rxjs-docs/blob/master/src/styles.scss#L1

Hmm, I'm getting build errors when I try to import it like this:

@import '~font-awesome/scss/font-awesome';

Any tips on how I should import it using this technique?

I will check

@sumitarora Got it, using this: https://stackoverflow.com/a/34366930/2521893

Both of these work it's not able to resolve the fonts which is why it's failing.
@import '~font-awesome/css/font-awesome.min.css';
@import './../node_modules/font-awesome/css/font-awesome.min.css';

It seems to work after these changes, doesn't it? fc9ca53

$fa-font-path: "~font-awesome/fonts"; @import "~font-awesome/scss/font-awesome";

sumitarora · 2017-12-13T20:40:25Z

package.json

    "@angular/router": "5.0.2",
    "@types/hammerjs": "2.0.35",
    "core-js": "2.4.1",
+    "font-awesome": "4.7.0",


Can't we use https://material.io/icons/ ?

The material icon set doesn't include any icons for social providers, such as linkedIn and Github, which I'm using on the team member cards

Ahha ok thanks 👍

Both of these work it's not able to resolve the fonts which is why it's failing.
@import '~font-awesome/css/font-awesome.min.css';
@import './../node_modules/font-awesome/css/font-awesome.min.css';

Also just for social icons, we are importing an entire icon set. Is there a subset that we can utilize ?

@ashwin-sureshkumar it looks like only the woff2 file is being pulled in anyway, so maybe it isn't a major issue for now. Could it possibly be fixed/replaced in another pr? Open an issue for looking into it?

@fredrik-lundin I agree let's deal with that in later PR

Sounds good. If there is a bit more research needed

I agree, this is an awfully big file to pull in for a single icon. We may need to look further at this before merging.

@btroncone , @sumitarora , @ashwin-sureshkumar please have a look at my latest commits. I removed FA and replaced it with a custom subset generated by https://icomoon.io/app. Much smaller footprint 👍

ladyleet · 2017-12-13T20:58:20Z

whoa y'all way to go on the hustle! thanks!

ashwin-sureshkumar · 2017-12-13T21:25:29Z

package.json

    "@angular/router": "5.0.2",
    "@types/hammerjs": "2.0.35",
    "core-js": "2.4.1",
+    "font-awesome": "4.7.0",


Also just for social icons, we are importing an entire icon set. Is there a subset that we can utilize ?

ashwin-sureshkumar · 2017-12-13T21:28:44Z

src/app/team/social-sharing/social-sharing.component.html

@@ -0,0 +1,9 @@
+<a *ngIf="!!githubUrl" mat-button [href]="githubUrl">
+  <mat-icon [ngStyle]="{'color': '#6f066f'}" fontSet="fa" fontIcon="fa-github"></mat-icon>


Please move the colors into scss and use classes. @knittingcodemonkey - please validate accessibility on the colors used

There are 3 ngIf - just a thought if this can be done without ngIf by handling the transformation in js.

ashwin-sureshkumar · 2017-12-13T21:31:48Z

src/app/team/member.component.html

+    <app-social-sharing
+      [githubUrl]="member.githubUrl"
+      [twitterUrl]="member.twitterUrl"
+      [linkedinUrl]="member.linkedinUrl">


email is missing here but its part of the member interface.

Fixed - removed that unused property from the model

ashwin-sureshkumar · 2017-12-13T21:33:47Z

@fredrik-lundin - Amazing job ! Welcome to open source !

knitcodemonkey

Please darken the linkedin icon color, and move styles to scss, please. This is great work!

knitcodemonkey · 2017-12-13T21:36:32Z

src/app/team/social-sharing/social-sharing.component.html

+  <mat-icon [ngStyle]="{'color': '#4073d4'}" fontSet="fa" fontIcon="fa-twitter"></mat-icon>
+</a>
+<a *ngIf="!!linkedinUrl" mat-button [href]="linkedinUrl">
+  <mat-icon [ngStyle]="{'color': '#4288b3'}" fontSet="fa" fontIcon="fa-linkedin"></mat-icon>


The other icon colors have a high contrast, but the LinkedIn icon is too light. Can we darken it to #3D7CA4?

Fixed in e889fac

+ use darker color for linkedIn, for accessability

fredrik-lundin · 2017-12-14T06:04:11Z

Rebased from updated upstream again. Let me know if there is anything else you want fixed before it's ready to go! :)

used https://icomoon.io to generate a much smaller font pack

ladyleet · 2017-12-18T21:51:49Z

@sumitarora @ashwin-sureshkumar are we still waiting on more changes or does all look good to go?

ashwin-sureshkumar · 2017-12-18T23:34:44Z

LGTM ! Except right now all the team members is filled with dummy data. So we need a real list compiled to go in. May be another PR or we can tackle it here. @ladyleet

Suggestion is to do it a different PR.

ladyleet · 2017-12-19T20:43:21Z

@ashwin-sureshkumar yeah let's do it with a new PR later.

ladyleet · 2017-12-19T20:43:46Z

oh pls tell me travis is not borked again

ladyleet · 2017-12-29T16:52:53Z

looks great thx @fredrik-lundin ! so happy the rxjs docs was your first OSS contribution! :)

fredrik-lundin added 7 commits October 16, 2017 22:32

feat(teams page): init wip

5234038

ReactiveX#14

feat(team page): split team page into smaller components

02a43aa

merge

fd98c64

feat(team page): center content

1e817a4

refactor(team page): remove unsued json files

2f1b8a9

fix(styles): remove style from bad merge

76eed5d

Merge branch 'master' into master

5c6a491

fix: remove unused html tag

ba1d966

should have been removed in previous merge

sumitarora suggested changes Dec 13, 2017

View reviewed changes

refactor: delete yarn-error.log

f995509

sumitarora reviewed Dec 13, 2017

View reviewed changes

refactor: import fa directly in styles.scss

fc9ca53

ashwin-sureshkumar suggested changes Dec 13, 2017

View reviewed changes

knitcodemonkey suggested changes Dec 13, 2017

View reviewed changes

fredrik-lundin added 4 commits December 13, 2017 22:42

fix(teams page): remove unused property on model

8f5248d

refactor(teams page): change mock name to Ben Lesh

fb41f87

refactor(teams pages): use classes for icon color

e889fac

+ use darker color for linkedIn, for accessability

rebase from upstream

83b5b00

sumitarora and others added 4 commits December 14, 2017 10:52

Merge branch 'master' into master

8efcfe3

feat(teams page): replace fa with individual fonts

caab113

used https://icomoon.io to generate a much smaller font pack

refactor(teams page): change name _fonts.scss to _icons.scss

e1d2148

Merge branch 'master' into master

5fbcfc4

ashwin-sureshkumar approved these changes Dec 18, 2017

View reviewed changes

sumitarora approved these changes Dec 19, 2017

View reviewed changes

sumitarora closed this Dec 19, 2017

sumitarora reopened this Dec 19, 2017

Merge branch 'master' into master

f59b1bc

ladyleet merged commit 802f7cb into ReactiveX:master Dec 29, 2017

		@@ -0,0 +1,7574 @@
		Arguments:
		C:\Program Files\nodejs\node.exe C:\Users\fredrikl\AppData\Roaming\npm\node_modules\yarn\bin\yarn.js

		@@ -0,0 +1,9 @@
		<a *ngIf="!!githubUrl" mat-button [href]="githubUrl">
		<mat-icon [ngStyle]="{'color': '#6f066f'}" fontSet="fa" fontIcon="fa-github"></mat-icon>

feat(teams page): Design a teams page #202

feat(teams page): Design a teams page #202

Uh oh!

Conversation

fredrik-lundin commented Dec 13, 2017

Uh oh!

fredrik-lundin commented Dec 13, 2017

Uh oh!

btroncone commented Dec 13, 2017

Uh oh!

sumitarora commented Dec 13, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fredrik-lundin Dec 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sumitarora Dec 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ashwin-sureshkumar Dec 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ladyleet commented Dec 13, 2017

Uh oh!

ashwin-sureshkumar Dec 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ashwin-sureshkumar Dec 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ashwin-sureshkumar commented Dec 13, 2017

Uh oh!

knitcodemonkey left a comment

Choose a reason for hiding this comment

Uh oh!

knitcodemonkey Dec 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

fredrik-lundin Dec 13, 2017 •

edited

Loading

sumitarora Dec 13, 2017 •

edited

Loading

ashwin-sureshkumar Dec 13, 2017 •

edited

Loading

ashwin-sureshkumar Dec 13, 2017 •

edited

Loading

ashwin-sureshkumar Dec 13, 2017 •

edited

Loading

knitcodemonkey Dec 13, 2017 •

edited

Loading

ashwin-sureshkumar commented Dec 18, 2017 •

edited

Loading