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

Wedding site - Final project Web Dev Bootcamp spring 2024 - Nathalie & Sofia #60

Open
wants to merge 147 commits into
base: main
Choose a base branch
from

Conversation

sofia32057
Copy link

Login info

[email protected]
secretwedding

Netlify link

https://project-wedding-site.netlify.app/

Collaborators

[ohitsnathalie]

sofia32057 and others added 30 commits May 26, 2024 16:17
@sofia32057 sofia32057 changed the title Wedding site - Final project Web Dev Bootcamp spring 2024 - Nahtalie & Sofia Wedding site - Final project Web Dev Bootcamp spring 2024 - Nathalie & Sofia Jun 24, 2024
Copy link

@HIPPIEKICK HIPPIEKICK left a comment

Choose a reason for hiding this comment

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

Hi Nathalie and Sofia and congratulations to completing this final project ⭐ I think the visuals of this project is what stands out. I know you struggled with some design choices but I think you really made it work in the end - it looks really good! The carousel, the FAQ, the parallax, the fonts, everything feels like it fits well together.

You've followed the requirements so I will approve your project, but please have a look at the comments I wrote to clean up, refactor and enhance this project. Apart from my comments I also want to mention something I noticed in lighthouse; that your Performance score is only at 75 and Best practices score is only at 74. I'd recommend you to get both of these up to a 90 at least, so have a look when you have time.

Keep up the good work and good luck in the future! 👋

Comment on lines +72 to +75

- This is a custom built website for this specific couple - not a "build your own wedding site" template. It will not have back office functionality, such as to upload/change content. This is managed in the code.
- Likewise, the guest list is managed in the database and the website will not include functionality to upload or handle the list client side.

Choose a reason for hiding this comment

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

Smart to include this in the readme 👍

Comment on lines +37 to +44
speech: {
isAllowed: {
type: Boolean,
},
willMakeSpeech: {
type: Boolean,
default: false,
},

Choose a reason for hiding this comment

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

This doesn't update when I check the box, is it because I'm not allowed as a tester? Maybe you'd want to change that so testers can test everything 🤓

Copy link
Author

Choose a reason for hiding this comment

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

It should update! Must be a bug. We'll have a look and fix it :)

Comment on lines +72 to +75
const guests = await Guest.find(
{},
"firstname lastname relation willAttend plusOne speech"
);

Choose a reason for hiding this comment

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

What does this do?

Choose a reason for hiding this comment

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

Why not include the thoughts in your backend as well? 👀

Copy link
Author

Choose a reason for hiding this comment

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

Time constraints - it's for sure on the To-do!

Comment on lines 12 to 42
"dependencies": {
"@headlessui/react": "^2.0.4",
"@heroicons/react": "^2.1.3",
"@tailwindcss/forms": "^0.5.7",
"moment": "^2.30.1",
"react": "^18.2.0",
"react-dom": "^18.2.0"
"react-dom": "^18.2.0",
"react-lazily": "^0.9.2",
"react-router-dom": "^6.23.1",
"react-router-hash-link": "^2.4.3",
"react-scroll-parallax": "^3.4.5",
"react-scroll-to-top": "^3.0.0",
"react-slick": "^0.30.2",
"slick-carousel": "^1.8.1",
"zustand": "^4.5.2"
},
"devDependencies": {
"@types/react": "^18.2.15",
"@types/react-dom": "^18.2.7",
"@vitejs/plugin-react": "^4.0.3",
"autoprefixer": "^10.4.19",
"eslint": "^8.45.0",
"eslint-plugin-react": "^7.32.2",
"eslint-plugin-react-hooks": "^4.6.0",
"eslint-plugin-react-refresh": "^0.4.3",
"vite": "^4.4.5"
"postcss": "^8.4.38",
"prettier": "^3.2.5",
"prettier-plugin-tailwindcss": "^0.5.14",
"tailwindcss": "^3.4.3",
"vite": "^5.2.11"
}

Choose a reason for hiding this comment

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

Would be nice if you'd include some information about these dependencies in your readme 👀

Choose a reason for hiding this comment

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

👍

import CoupleImage from "/pexels-sandeep-malith-97607821-9361349.webp";
import CoupleImage2 from "/pexels-asadphoto-1024984.webp";

export const FishtankSection = () => {

Choose a reason for hiding this comment

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

Is this a common saying that I've missed 😅

Comment on lines +3 to +14
export const Footer = () => {
return (
<div className="flex flex-col items-end justify-end gap-6 px-6 py-5 text-right text-sm md:flex-row ">
{/* <Button label={"RSVP"} type={"link"} style={"primary"} href={"/rsvp"} /> */}
<p className="font-cormorant">
©2024 created by{" "}
<a href="https://sofias-portfolio.netlify.app/">Sofia</a> &{" "}
<a href="https://portfolio-nathalie.netlify.app/">Nathalie</a>
</p>
</div>
);
};

Choose a reason for hiding this comment

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

The footer is covered by the "Up top"-arrow. And please make links open in a new tab

Comment on lines +8 to +9
const API_ENDPOINT =
"https://project-happy-thoughts-api-4mf8.onrender.com/thoughts";

Choose a reason for hiding this comment

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

You could have endpoints and such as env variables

Choose a reason for hiding this comment

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

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