Page MenuHomePhabricator
Differential D5390

[landing] populated investor page with investor data
ClosedPublic
Actions

Authored by ginsu on Oct 19 2022, 11:20 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Mar 25, 7:12 PM
Unknown Object (File)
Thu, Mar 20, 8:14 AM
Unknown Object (File)
Tue, Mar 11, 7:03 AM
Unknown Object (File)
Feb 24 2025, 3:21 AM
Unknown Object (File)
Feb 21 2025, 8:08 PM
Unknown Object (File)
Feb 16 2025, 8:40 PM
Unknown Object (File)
Feb 12 2025, 7:15 PM
Unknown Object (File)
Feb 12 2025, 7:15 PM
View All Files
Subscribers
• abosh
ashoat
atul
tomek

Details

Reviewers
atul
rohan
ashoat
Commits
rCOMMa19884a64fa8: [landing] populated investor page with investor data
Summary

last diff in the stack! let's gooo! populated the data from investor-data.js mapped the data into it's own investor profile card. When an investor profile card is clicked, the id of the investor is passed to the InvestorProfileModal as a prop then the InvestorProfileModal finds the investor in the data and displays it in the modal

Linear Task: ENG-1953
Figma: Investor Page

Depends on D5389 and D5391

Test Plan

Please watch the demo video to see how the investor page looks/performs:

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

ginsu created this revision.Oct 19 2022, 11:20 AM
Herald added subscribers: • abosh, atul, tomek, ashoat. · View Herald TranscriptOct 19 2022, 11:20 AM
ginsu edited the summary of this revision. (Show Details)Oct 19 2022, 11:20 AM
ginsu edited the test plan for this revision. (Show Details)
ginsu edited the summary of this revision. (Show Details)Oct 19 2022, 11:24 AM
ginsu edited the test plan for this revision. (Show Details)
ginsu added reviewers: atul, rohan.
ginsu added a parent revision: D5389: [landing] implemented investor profile modal UI design.
ginsu attached a referenced file: F203296: Screen Recording 2022-10-19 at 2.21.51 PM.mov. (Show Details)
Harbormaster completed remote builds in B12871: Diff 17662.Oct 19 2022, 11:32 AM
ginsu requested review of this revision.Oct 19 2022, 11:32 AM
atul requested changes to this revision.Oct 19 2022, 11:42 AM

Let's create an object using lodash keyBy (https://lodash.com/docs/#keyBy) and index into that instead of iterating over the entire investors array every time an <InvestorProfile> is clicked on

landing/investor-profile-modal.react.js
21–23 ↗(On Diff #17662)

It's not ideal that we're iterating over every element to find one with matching id (O(n)) every time the user clicks on an investor.

What we can do instead is export an object (maybe called keyedInvestorData) from ./investorData created with the lodash keyBy utility: https://lodash.com/docs/#keyBy. Then we'll be able to index into that object in constant time (O(1)).

Let me know if you need help with this, but should be pretty simple change.

landing/investors.react.js
28 ↗(On Diff #17662)

We can use React.useCallback()

This revision now requires changes to proceed.Oct 19 2022, 11:42 AM
ginsu updated this revision to Diff 17667.Oct 19 2022, 1:13 PM
ginsu edited the summary of this revision. (Show Details)

addressed atul's comments

ginsu edited the summary of this revision. (Show Details)Oct 19 2022, 1:16 PM
ginsu added a parent revision: D5391: [landing] added lodash dependency.
Harbormaster completed remote builds in B12876: Diff 17667.Oct 19 2022, 1:25 PM
atul accepted this revision.EditedOct 19 2022, 1:45 PM
atul added a reviewer: ashoat.

Looks good, let's early exit where I left a comment and it should be ready to go.

Adding @ashoat as blocking reviewer since it's public facing change

landing/investor-profile-modal.react.js
21–33 ↗(On Diff #17667)

We can early exit

Context: https://www.notion.so/commapp/Always-exit-early-9df0993fda5e41fd9c6e5e9a2248436d

ginsu updated this revision to Diff 17668.Oct 19 2022, 1:53 PM

address atul's exit early feedback

Harbormaster completed remote builds in B12877: Diff 17668.Oct 19 2022, 2:04 PM
ashoat accepted this revision.Oct 20 2022, 5:13 AM

Video looks great!!

last diff in the stack!

We're thinking of adding Twitter / LinkedIn links in a follow-up diff, yeah?

This revision is now accepted and ready to land.Oct 20 2022, 5:13 AM
ginsu added a comment.Oct 20 2022, 6:58 AM
In D5390#159696, @ashoat wrote:

We're thinking of adding Twitter / LinkedIn links in a follow-up diff, yeah?

Yeah, I wrote that in the description before we talked about the Twitter/LinkedIn links, but I just designed and submitted a potential design for the Investor Profile cards with those icons on linear

ginsu added a child revision: D5449: [landing] added social links to investor cards.Oct 20 2022, 1:18 PM
Closed by commit rCOMMa19884a64fa8: [landing] populated investor page with investor data (authored by ginsu). · Explain WhyOct 21 2022, 4:58 PM
This revision was automatically updated to reflect the committed changes.
ginsu added a commit: rCOMMa19884a64fa8: [landing] populated investor page with investor data.

Revision Contents
Changeset List

Diff 17837

View Options

landing/investor-data.js

Show All 21 Lines
a { a {
text-decoration: none; text-decoration: none;
color: #2a5db0; color: #2a5db0;
} }
img, img,
iframe { iframe {
display: block; display: block;
} }
input[type='text'], input[type='text'],
input[type='password'], input[type='password'],
textarea { textarea {
atulUnsubmitted
Not Done
Inline Actions

Moving the global input styling and moving it to the input component.

Then why are we keeping styles like border and border-radius?

Don't these styles also affect the <textarea> tag? Are you sure we want to be removing these styles for that as well? Or should we break this into two separate selectors (one for input and another for textarea)?

atul: > Moving the global input styling and moving it to the input component. Then why are we…
benschacAuthorUnsubmitted
Done
Inline Actions

Yeah, I should break these styles out a bit more. I don't think they really do anything. But you make a good point that this will also effect textarea elements.

benschac: Yeah, I should break these styles out a bit more. I don't think they really do anything. But…
-webkit-appearance: none;
-moz-appearance: none;
-webkit-border-radius: 0;
border: 1px solid #dddddd; border: 1px solid #dddddd;
border-radius: 1px; border-radius: 1px;
font-family: var(--font-stack); font-family: var(--font-stack);
} }
:global(#react-root) { :global(#react-root) {
display: flex; display: flex;
flex-direction: column; flex-direction: column;
height: 100%; height: 100%;
▲ Show 20 Lines • Show All 243 Lines • Show Last 20 Lines
View Options

landing/investor-profile-modal.react.js

Show All 21 Lines
a { a {
text-decoration: none; text-decoration: none;
color: #2a5db0; color: #2a5db0;
} }
img, img,
iframe { iframe {
display: block; display: block;
} }
input[type='text'], input[type='text'],
input[type='password'], input[type='password'],
textarea { textarea {
atulUnsubmitted
Not Done
Inline Actions

Moving the global input styling and moving it to the input component.

Then why are we keeping styles like border and border-radius?

Don't these styles also affect the <textarea> tag? Are you sure we want to be removing these styles for that as well? Or should we break this into two separate selectors (one for input and another for textarea)?

atul: > Moving the global input styling and moving it to the input component. Then why are we…
benschacAuthorUnsubmitted
Done
Inline Actions

Yeah, I should break these styles out a bit more. I don't think they really do anything. But you make a good point that this will also effect textarea elements.

benschac: Yeah, I should break these styles out a bit more. I don't think they really do anything. But…
-webkit-appearance: none;
-moz-appearance: none;
-webkit-border-radius: 0;
border: 1px solid #dddddd; border: 1px solid #dddddd;
border-radius: 1px; border-radius: 1px;
font-family: var(--font-stack); font-family: var(--font-stack);
} }
:global(#react-root) { :global(#react-root) {
display: flex; display: flex;
flex-direction: column; flex-direction: column;
height: 100%; height: 100%;
▲ Show 20 Lines • Show All 243 Lines • Show Last 20 Lines
View Options

landing/investors.react.js

Show All 21 Lines
a { a {
text-decoration: none; text-decoration: none;
color: #2a5db0; color: #2a5db0;
} }
img, img,
iframe { iframe {
display: block; display: block;
} }
input[type='text'], input[type='text'],
input[type='password'], input[type='password'],
textarea { textarea {
atulUnsubmitted
Not Done
Inline Actions

Moving the global input styling and moving it to the input component.

Then why are we keeping styles like border and border-radius?

Don't these styles also affect the <textarea> tag? Are you sure we want to be removing these styles for that as well? Or should we break this into two separate selectors (one for input and another for textarea)?

atul: > Moving the global input styling and moving it to the input component. Then why are we…
benschacAuthorUnsubmitted
Done
Inline Actions

Yeah, I should break these styles out a bit more. I don't think they really do anything. But you make a good point that this will also effect textarea elements.

benschac: Yeah, I should break these styles out a bit more. I don't think they really do anything. But…
-webkit-appearance: none;
-moz-appearance: none;
-webkit-border-radius: 0;
border: 1px solid #dddddd; border: 1px solid #dddddd;
border-radius: 1px; border-radius: 1px;
font-family: var(--font-stack); font-family: var(--font-stack);
} }
:global(#react-root) { :global(#react-root) {
display: flex; display: flex;
flex-direction: column; flex-direction: column;
height: 100%; height: 100%;
▲ Show 20 Lines • Show All 243 Lines • Show Last 20 Lines