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

Uploaded all the images of the previous website and the base template of the last site's homepage #9

Closed
wants to merge 5 commits into from

Conversation

DaiseyD
Copy link
Collaborator

@DaiseyD DaiseyD commented Jul 10, 2024

Maybe it is worth it to restructure the folders in this, but right now, this seemed like an okay start.

Copy link
Collaborator

@falsepopsky falsepopsky left a comment

Choose a reason for hiding this comment

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

I still need to take a closer look. I'm going to set up prettier in another pull request so you can format the files.

Copy link
Collaborator

@falsepopsky falsepopsky Jul 10, 2024

Choose a reason for hiding this comment

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

use pnpm for install
corepack enable & pnpm install

@@ -16,7 +19,21 @@ export default function RootLayout({
}>) {
return (
<html lang="en">
<body className={inter.className}>{children}</body>
<head>
<meta charSet="utf-8"/>
Copy link
Collaborator

Choose a reason for hiding this comment

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

do you have experience with Next.js?

yo should use:

https://nextjs.org/docs/app/building-your-application/optimizing/metadata

@@ -0,0 +1,345 @@
* {
Copy link
Collaborator

Choose a reason for hiding this comment

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

tailwind already handles most of this., so some styles classes should be removed

Comment on lines +34 to +35
<body className={inter.className}>
<Navbar></Navbar>
Copy link
Collaborator

Choose a reason for hiding this comment

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

it should be <Navbar />

<nav>
<ul className="flex justify-between text-2xl">
<li className="mr-6">
<a className="text-blue-500 hover:text-blue-800" href="/">Home</a>
Copy link
Collaborator

Choose a reason for hiding this comment

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

</a>
return ( <div>
<header>
<img src="img/logo.png" width="500" height="500" alt="Retro Olympics"/>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Comment on lines +10 to +12
<h3>The 2023-2024 season is coming!</h3>
<h2>Gather your team and prepare for the...</h2>
<h1>RETRO OLYMPICS!</h1>
Copy link
Collaborator

Choose a reason for hiding this comment

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

if this is a text you should use the p tag instead, don't use headers for non titles

@DaiseyD DaiseyD closed this Jul 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants