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

About us page and group pages #62

Open
wants to merge 40 commits into
base: dev
Choose a base branch
from
Open

About us page and group pages #62

wants to merge 40 commits into from

Conversation

supattrw
Copy link

these is the pages that is worth to take a look at ...??

  • about
  • devops
  • member representative
  • leaderboard

not done (but technocally same layout)

  • breadboard
  • game
  • ttrpg

under leader and deputy leader (in devops etc.), Im thinking of using Lisa's member-component.
please give me a better solution to create a table for 'list of all members'
the accordion component is meant to look like the one in aboutPage
the FAQ's is not done

Please be kind 😭😭😭

@michaelbrusegard
Copy link
Member

Im gonna fix commit messages real quick. Next time dont use norwegian and use the conventional commit standard on all messages :)

@michaelbrusegard
Copy link
Member

michaelbrusegard commented Oct 20, 2024

Cant do it because they are signed...

Fixing it by installing lazygit on your pc

@michaelbrusegard
Copy link
Member

We were not able to fix all commit messages unfortunately but that is fine

README.md Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
messages/en.json Outdated Show resolved Hide resolved
messages/en.json Outdated Show resolved Hide resolved
messages/en.json Outdated Show resolved Hide resolved
src/app/[locale]/(default)/about/page.tsx Outdated Show resolved Hide resolved
src/app/[locale]/(default)/about/page.tsx Outdated Show resolved Hide resolved
src/components/ui/Meteor.tsx Outdated Show resolved Hide resolved
src/components/ui/Meteor.tsx Outdated Show resolved Hide resolved
src/lib/locale/index.ts Outdated Show resolved Hide resolved
@michaelbrusegard
Copy link
Member

The biggest issue is that you are not reusing components. Try to create more files of smaller building blocks

@michaelbrusegard michaelbrusegard changed the title About us page and other pages About us page and group pages Oct 20, 2024
@michaelbrusegard
Copy link
Member

@supattrw IF you want you can focus on the main about page in this PR and the group pages can be separate PRs. I think that would be a very good idea so other people can work on the individual pages too as they finish other parts of the website

@ZeroWave022
Copy link
Member

Waiting until all of Michael's comments are resolved before submitting my own review.

@@ -36,7 +36,6 @@
"navigationMenu": "Navigation menu",
"news": "News",
"events": "Events",
"storage": "Storage",
Copy link
Member

Choose a reason for hiding this comment

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

Revert change

@@ -45,7 +46,7 @@
"cva": "^1.0.0-beta.1",
"date-fns": "^4.1.0",
"drizzle-orm": "^0.33.0",
"lucia": "3.2.0",
"lucia": "^3.2.0",
Copy link
Member

Choose a reason for hiding this comment

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

Revert change

Comment on lines +9 to +14
const images = [
{ id: "image1", src: "/unknown.png", alt: "unknown" },
{ id: "image2", src: "/mock.jpg", alt: "mock" },
{ id: "image3", src: "/bg.jpg", alt: "bg" },
{ id: "image4", src: "/about/pizzaWolfs-min.png", alt: "pizzaWolfs" },
];
Copy link
Member

Choose a reason for hiding this comment

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

Alts for images should be descriptive

const t = useTranslations('labops');

return (
<div>
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't look like the div is strictly necessary here

Comment on lines +15 to +16
<h1 className='mt-4 mb-4 w-full'> DevOps </h1>
<div className='flex w-full'> {t('about')} </div>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<h1 className='mt-4 mb-4 w-full'> DevOps </h1>
<div className='flex w-full'> {t('about')} </div>
<h1 className='mt-4 mb-4 w-full'>DevOps</h1>
<div className='flex w-full'>{t('about')}</div>

<div>
<h3 className='mb-2'> {t('financialManager')} </h3>
<Card className='p-20'>
<CardHeader> Økonomiansvarlig </CardHeader>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<CardHeader> Økonomiansvarlig </CardHeader>
<CardHeader>Økonomiansvarlig</CardHeader>


return (
<div>
<h1 className='mt-4 flex w-full items-center justify-center dark:text-primary'> Member Representative </h1>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<h1 className='mt-4 flex w-full items-center justify-center dark:text-primary'> Member Representative </h1>
<h1 className='mt-4 flex w-full items-center justify-center dark:text-primary'>Member Representative</h1>

Comment on lines +75 to +78
p1: (chunks) => <p className="p1"> {chunks} </p>,
p2: (chunks) => <p className="p2"> {chunks} </p>,
p3: (chunks) => <p className="p3"> {chunks} </p>
})}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
p1: (chunks) => <p className="p1"> {chunks} </p>,
p2: (chunks) => <p className="p2"> {chunks} </p>,
p3: (chunks) => <p className="p3"> {chunks} </p>
})}
p1: (chunks) => <p className="p1">{chunks}</p>,
p2: (chunks) => <p className="p2">{chunks}</p>,
p3: (chunks) => <p className="p3">{chunks}</p>
})}

Comment on lines +12 to +20
type images = {
id: string;
src: string;
alt: string;
};

type ImageCarouselProps = {
images: images[];
};
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
type images = {
id: string;
src: string;
alt: string;
};
type ImageCarouselProps = {
images: images[];
};
type ImageCarouselProps = {
images: {
id: string;
src: string;
alt: string;
}[];
};

Comment on lines +9 to +16
type Member = {
name: string;
image: string;
};

type MembersTableProps = {
Members: Member[];
};
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
type Member = {
name: string;
image: string;
};
type MembersTableProps = {
Members: Member[];
};
type MembersTableProps = {
members: {
name: string;
image: string;
}[];
};

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