Skip to content
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

Update the mobile take home project site based off of the proposed RF… #5

Merged
merged 1 commit into from
Jan 14, 2022

Conversation

Blisse
Copy link
Collaborator

@Blisse Blisse commented Jan 12, 2022

…C improvements

RFC: https://docs.google.com/document/d/1Yu1bcg9WrqsnNCIWuI38Y9QBFy8oIEYAxgBl2q4FQKM/edit#

Had to make some additional wording changes to make things flow a bit better.

@Blisse
Copy link
Collaborator Author

Blisse commented Jan 12, 2022

127 0 0 1_4000_mobile-interview-project_

Here's a screenshot of the website

@sandres-square
Copy link
Contributor

PR 5 is content-based and I take no opinion on that (you folks own the content). If someone else on the team wants to approve, I'm happy to merge


Take care to properly handle errors returned from the endpoint (or other network errors like timeouts), and ensure you do not waste network bandwidth – load expensive resources such as large photos on-demand only. Photos at a given URL will never change. Once one is loaded, you do not need to reload the photo. If an employee’s photo changes, they will be given a new photo URL (eg, you can treat the URL as a GUID).
Build an employee directory app that shows a list of employees from the provided endpoint.
Copy link
Contributor

Choose a reason for hiding this comment

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

endpoint -> endpoints

Copy link
Contributor

Choose a reason for hiding this comment

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

Though i guess this is fine if we consider the other endpoints just for testing!

Copy link
Contributor

@kyleve kyleve left a comment

Choose a reason for hiding this comment

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

Looks really good! Just some feedback / grammar. Thanks for driving this!

@Blisse Blisse force-pushed the victorlai.update-take-home branch 2 times, most recently from 281ab5a to f417151 Compare January 13, 2022 05:16
@Blisse Blisse changed the base branch from victorlai.update-site to main January 13, 2022 19:11
@Blisse
Copy link
Collaborator Author

Blisse commented Jan 13, 2022

Fixed one more grammar error

@sandres-square sandres-square merged commit dd9f2e7 into main Jan 14, 2022
@akambale akambale deleted the victorlai.update-take-home branch June 19, 2024 18:39
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.

3 participants