Skip to content

Epic/cv jedi minimalism #283 create contact section #338

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

Open
wants to merge 7 commits into
base: epic/cv-jedi-minimalism
Choose a base branch
from

Conversation

AbelDeTena
Copy link
Collaborator

No description provided.

@AbelDeTena AbelDeTena changed the base branch from feature/develop to epic/cv-jedi-minimalism January 16, 2024 13:05
@AbelDeTena AbelDeTena closed this Jan 16, 2024
@AbelDeTena AbelDeTena reopened this Jan 16, 2024
@@ -0,0 +1,98 @@
<%_ if (profile.relevantLinks && profile.relevantLinks !== 0) { -%>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (profile.relevantLinks && profile.relevantLinks.length > 0)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replaced

/>
</svg>
</div>
<a href="tel:+#">+1 041 234 5678</a>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we have phone display phone. Add a condition and use props to display phone

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replace by <%=link.URL%>

/>
</svg>
</div>
<a href="mailto:#">[email protected]</a>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same here

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replace by <%=link.URL%>

</defs>
</svg>
</div>
<a href="#" target="_blank">https://linkedin.com/in/david-bonilla</a>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the same

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replace by <%=link.URL%>

/>
</svg>
</div>
<a href="#" target="_blank">https://github.com/dbonilla</a>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the same

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replace by <%=link.URL%>

/>
</svg>
</div>
<a href="#" target="_blank">https://twitter/david-bonilla</a>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the same

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replace by <%=link.URL%>

/>
</svg>
</div>
<a href="#" target="_blank">https://www.david-bonilla.com</a>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the same

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replace by <%=link.URL%>

@AbelDeTena AbelDeTena force-pushed the epic/cv-jedi-minimalism-#283-create-contact-section branch from c1d5c26 to 55dbc95 Compare January 18, 2024 16:45
@@ -0,0 +1,117 @@
<%_ if (profile.relevantLinks && profile.relevantLinks.length > 0 || profile.phoneNumbers && profile.phoneNumbers.length > 0 ||profile.emails && profile.emails.length > 0 || profile.city && profile.country ) { -%>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is wrong. You can get the country but not the city. Replace by:

<%_ if (profile.relevantLinks && profile.relevantLinks.length > 0 || profile.emails && profile.emails.length > 0 || profile.city || profile.country || profile.phoneNumbers && profile.phoneNumbers.length > 0) { -%>


<h2>Contact</h2>
<div class="aside__container">
<%_ for (const number of profile?.phoneNumbers) { -%>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two thinks:

  • What happen here if we have relevantLinks por example, but not phoneNumbers?
  • profile.phoneNumbers is an array, number is an item in the array

You change this, if we have phoneNumbers we display phones

/>
</svg>
</div>
<a href="tel:+#"><%=profile.phoneNumbers[0].countryCode%> <%=profile.phoneNumbers[0].number%></a>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It`s wrong: number.countryCode and number.phoneNumber

<%_ } %>
<%_ } -%>

<%_ for (const email of profile?.emails) { -%>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same here

<%_ } %>
<%_ } -%>

<%_ if (profile.city && profile.country) { -%>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's wrong

</div>
<%_ } %>

<%_ for (const link of profile?.relevantLinks) { -%>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happen here if we don't have relevantLinks?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refer to the file relevants-links-section.ejs in cv-monochrome-force and try to correct this file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants