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

I27 create a contact page #49

Merged
merged 132 commits into from Jul 16, 2022
Merged

I27 create a contact page #49

merged 132 commits into from Jul 16, 2022

Conversation

Will-H007
Copy link
Collaborator

@Will-H007 Will-H007 commented Jun 27, 2022

Change Summary

[Briefly summarise the changes that you made. Just high-level stuff]

Change Form

Fill this up (NA if not available). If a certain criteria is not met, can you please give a reason.

  • The pull request title has an issue number
  • The change works by "Smoke testing" or quick testing
  • The change has tests
  • The change has documentation

Other Information

[Is there anything in particular in the review that I should be aware of?]

contactinfo.tsx

  • The phone icon will call the attached number
  • The mail icon opens up email
  • The marker icon opens up google maps with address
  • Added an edit icon (not on figma design) which switches elements between input fields and text fields.
  • Added save and cancel edit buttons, save will submit the new data, cancel will keep the original data

contact/[id].tsx

  • Currently uses getStaticProps/Path this will need to be changed when data is retrieved from the API

ContactItem & ContactInfo

  • retrieves the contact image as one of the props, if we finish the MVP and continue working on it the image should be a part of the Contact type. Our code is already made to handle an image.

ContactForm

  • When the fields update it is captured by the state contactForm. ContactForm holds the data that will be sent to firebase.

Related Issue

@vercel
Copy link

vercel bot commented Jun 27, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
poops ✅ Ready (Inspect) Visit Preview Jul 15, 2022 at 1:03AM (UTC)

@ChantelleKerr ChantelleKerr changed the title I27 create a contact page WIP I27 create a contact page Jun 28, 2022
src/components/Contact/contactlist.tsx Outdated Show resolved Hide resolved
Copy link
Collaborator

@APM246 APM246 left a comment

Choose a reason for hiding this comment

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

Great work, some random stuff I noticed (in addition to in-line comments):

  • Cursor icon doesn't change when hovering over edit button
  • On smaller screens, when a contact page has several pets/notes there is no margin on the bottom element (e.g. the "Me" contact page)
  • for better clarity the pages/contact.tsx file can be moved to pages/contact/index.tsx
  • Is new-tab.png used anywhere?
  • don't have to do this but maybe it would be a good idea to add a //TODO anywhere CONTACT_DATA is used

src/pages/contact.tsx Outdated Show resolved Hide resolved
src/components/Contact/contactform.tsx Outdated Show resolved Hide resolved
src/components/Contact/contactform.tsx Outdated Show resolved Hide resolved
@ChantelleKerr
Copy link
Contributor

ChantelleKerr commented Jul 12, 2022

@APM246

  • Cursor icon doesn't change when hovering over edit button

DONE

  • On smaller screens, when a contact page has several pets/notes there is no margin on the bottom element (e.g. the "Me" contact page)

DONE

  • Is new-tab.png used anywhere?

DONE - I removed it because it's not used

  • don't have to do this but maybe it would be a good idea to add a //TODO anywhere CONTACT_DATA is used

DONE

Thanks for reviewing I'll get to the other requested changes later tonight :)

@zachpmanson
Copy link
Collaborator

All requested changes have been fixed

APM246
APM246 previously approved these changes Jul 14, 2022
Copy link
Collaborator

@APM246 APM246 left a comment

Choose a reason for hiding this comment

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

Should be good to merge once merge conflicts are resolved :)

Copy link
Member

@NicholasChoong NicholasChoong left a comment

Choose a reason for hiding this comment

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

Looks good!

Copy link
Member

@skyheat skyheat left a comment

Choose a reason for hiding this comment

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

Looks good!

@NicholasChoong NicholasChoong merged commit 2d0ea94 into main Jul 16, 2022
@NicholasChoong NicholasChoong deleted the i27-create_a_contact_page branch July 16, 2022 03:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frontend Task must have a front end issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create a Contact page
7 participants