Skip to content

Security Page #258

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

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Conversation

kartik-vats
Copy link

No description provided.

@geekysilento geekysilento changed the base branch from master to feature/security-section February 5, 2025 15:29
@heydoyouknowme0
Copy link
Contributor

#248

@geekysilento
Copy link
Member

You didnt change anything from suyash's PR, were you not informed?

<Card className="bg-white mx-4 border-[#971F16] bg-neutral-50 py-4 font-sans font-medium tracking-wide sm:mx-6 md:mx-10 lg:mx-24">
<CardContent>
<CardDescription className="text-black mb-4 line-clamp-6">
{text.card2.content1}
Copy link
Member

Choose a reason for hiding this comment

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

use a .map() instead of doing this repeatedly

></ImageHeader>

<section className="mt-10 px-4 sm:px-6 md:px-10 lg:px-24">
<Card className=" my-0 border-none py-4 font-sans text-xs font-semibold tracking-wide shadow-none sm:text-xl md:text-2xl lg:text-[40px]">
Copy link
Member

Choose a reason for hiding this comment

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

Don't use <Card>, see how these type of components are made in other pages, like /institute/sections/library/page.tsx

<CardDescription className="text-black leading-[20px] font-sans text-justify mb-4">
{text.article.description3}
</CardDescription>
<CardDescription className="text-black leading-[20px] font-sans text-justify mb-4">
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
<CardDescription className="text-black leading-[20px] font-sans text-justify mb-4">
<CardDescription className="leading-[20px] font-sans text-justify mb-4">

dont use text colors that aren't already custom defined in our color palette in the tailwind.config.ts file

</Card>
</section>

<section className="mt-12 ">
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
<section className="mt-12 ">
<section className="mt-12">

you probably arent formatting ur code properly, ensure Prettier is configured and running.

/>
</article>

<Card className="mx-4 my-0 border-[#971F16] bg-neutral-50 py-4 tracking-wide sm:mx-6 md:mx-10 lg:mx-24">
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
<Card className="mx-4 my-0 border-[#971F16] bg-neutral-50 py-4 tracking-wide sm:mx-6 md:mx-10 lg:mx-24">
<Card className="mx-4 my-0 border-primary-500 bg-neutral-50 py-4 tracking-wide sm:mx-6 md:mx-10 lg:mx-24">

again, dont use hardcoded values, use our color palette

Copy link
Member

Choose a reason for hiding this comment

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

this file shouldnt be part of ur PR, why is my commit a part of it, remove the entire commit from ur PR

i18n/en.ts Outdated
Security: {
title: 'SECURITY SECTION',
article: {
description1:
Copy link
Member

Choose a reason for hiding this comment

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

this should be an array instead of doing description 1,2,3 etc

Comment on lines 330 to 333
description1: string;
description2: string;
description3: string;
description4: string;
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
description1: string;
description2: string;
description3: string;
description4: string;
description: string[];

this should be an array and shown using the .map function as mentioned earlier

@geekysilento geekysilento linked an issue Feb 6, 2025 that may be closed by this pull request
@heydoyouknowme0 heydoyouknowme0 changed the base branch from feature/security-section to master February 6, 2025 17:21
@geekysilento geekysilento requested a review from Copilot April 10, 2025 16:48
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

app/[locale]/institute/sections/security/page.tsx:14

  • The translations are defined with 'Security' at the root level in the translation files, but the code accesses 'Section.Security'. Consider updating the property access to match the translations structure.
const text = (await getTranslations(locale)).Section.Security;

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.

Security Page
3 participants