-
Notifications
You must be signed in to change notification settings - Fork 51
Use a dialog to display the LargePortraitCard. #169
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use a dialog to display the LargePortraitCard. #169
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
4a056b3
to
c56cda8
Compare
c56cda8
to
4f3d386
Compare
c269866
to
5b58c1b
Compare
dfb33bd
to
207e124
Compare
207e124
to
76faa10
Compare
26f909e
to
74ae9c6
Compare
1bcc8b2
to
3756b2d
Compare
c893c09
to
ea5a66f
Compare
ea5a66f
to
8b48f2d
Compare
8b48f2d
to
0b65a23
Compare
d0a34c0
to
b74d3a8
Compare
b74d3a8
to
50aab1e
Compare
render={({ history, match }) => { | ||
const { completeName } = match.params; /* extract the dynamic part from the url i.e. the completeName*/ | ||
const teamMembers = getTeamByPersonName(completeName); | ||
const person = teamMembers.find((person) => person.completeName.replace(/\s+/g, '').normalize("NFD").replace(/[\u0300-\u036f]/g, '') === completeName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because you're doing this name string clean up step twice (with the replace(...)
etc.), I would suggest either storing the cleaned up version as part of the person's JSON blob or writing a small utility function so that you're not duplicating this logic. (not a blocker for merging)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this file? It seems like something that was not supposed to be in the PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be fair, I don't know.
@afshin Thanks for the review. I will take your comment into account |
… the dialog by clicking the overlay.
…es for the collaborators.
…act that the individual popup were not rendered on /about/person page when entering the corresponding url.
…ters. Add logics to not scroll systematically to the top of the page when changing from about page to portrait ones and reciprocally.
Co-authored-by: Afshin Taylor Darian <[email protected]>
8cca664
to
c1a4bf9
Compare
c1a4bf9
to
68cfaba
Compare
68cfaba
to
fcf6cf7
Compare
Use a dialog to display the LargePortraitCard.
Should fix #129