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

Thanks! #1

Open
ledaian41 opened this issue Jul 23, 2019 · 3 comments
Open

Thanks! #1

ledaian41 opened this issue Jul 23, 2019 · 3 comments

Comments

@ledaian41
Copy link

Hi @TanDung2512, thanks for your submission.

Things you did well:

  • Naming variable in camelCase, meaning name.
  • Very well formatted and readable code.

Things to improve:

  • Shouldn't nest <script> and <style> inside html body
  • Should naming id be more meaningful, instead of "number"
  • Make UI more beautiful with css.

Excellent work! You are demonstrating that you are understanding the material and doing a great job of applying it. Keep it up!

@ledaian41
Copy link
Author

Hi @TanDung2512, thanks for your submission.

Things you did well:

  • Good coding convention.
  • Using StyleSheet, ScrollView, TouchableOpacity.
  • Has shadow and effect for buttons
  • Beautiful photos.

Things to improve:

  • Try to separate app screen into small components, organize folder structure of project.
  • Draw the avatar to be circle.
  • Improve UI, spacing margin top is not good. At the bottom, images are not displayed fully. Does it work fine on your testing devices?
  • 3 lines for Name, Description and Follow, Send buttons are longer than Avatar image. They should have same height with Avatar

Excellent work! You are demonstrating that you are understanding the material and doing a great job of applying it. Keep it up!

@ledaian41
Copy link
Author

ledaian41 commented Aug 11, 2019

Hi @TanDung2512, thanks for your submission.

Required features are implemented very well.

Things to improve:

  • Complete and Active screens didn't work correctly, data todo list are not synchronized with All Todo screen.

Thank you,

@ledaian41
Copy link
Author

Hi @TanDung2512, thank you for your submission

Things to improve:

  • Issue when scrolling down to the end of the list, it fetch more articles but re-render all list and display again from top of the list

Thank you,

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

No branches or pull requests

1 participant