Skip to content

made some changes to the arbitrary pages#3

Open
SimpleX-T wants to merge 1 commit intomasterfrom
admin/idan-devs-branch
Open

made some changes to the arbitrary pages#3
SimpleX-T wants to merge 1 commit intomasterfrom
admin/idan-devs-branch

Conversation

@SimpleX-T
Copy link
Copy Markdown
Collaborator

I moved the auth pages to the section with header and footer.
I made some changes to the arbitrary pages (/docs, /guidlines...), making them rhyme with the pages look

@SimpleX-T SimpleX-T requested a review from Copilot May 25, 2025 16:00
Copy link
Copy Markdown
Contributor

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.

Pull Request Overview

This PR refactors page layouts by moving authentication pages into the shared header/footer wrapper and standardizing the outer container of several static pages, plus it enriches the Documentation page with structured content.

  • Removed debug logging from the forgot-password form
  • Wrapped /terms, /privacy, and /guidelines pages in full-screen <section> containers
  • Expanded the /docs page with Overview, Technical Stack, Authentication, and Design System sections

Reviewed Changes

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

Show a summary per file
File Description
components/auth/forgot-password-form.tsx Removed console.log used for debugging
app/not-found.tsx Adjusted whitespace around useRouter import
app/(with-header-and-footer)/terms/page.tsx Replaced inner <div> with <section> for consistent layout
app/(with-header-and-footer)/privacy/page.tsx Same container-to-section refactor as TermsPage
app/(with-header-and-footer)/guidelines/page.tsx Same container-to-section refactor as TermsPage
app/(with-header-and-footer)/docs/page.tsx Switched to <section>, added detailed content cards

Comment on lines 4 to 9
<h1 className="text-3xl font-semibold mb-6">Terms of Service</h1>
<div className="bg-card p-6 rounded-lg shadow-md">
<p className="text-muted-foreground">
This is a placeholder for the terms of service page. Our terms and
conditions will be detailed here soon.
</p>
Copy link

Copilot AI May 25, 2025

Choose a reason for hiding this comment

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

[nitpick] This section wrapper removes the max-width constraint, which may lead to overly wide content on large screens. Consider wrapping the inner content in a centered container (e.g., max-w-4xl mx-auto) for better readability.

Suggested change
<h1 className="text-3xl font-semibold mb-6">Terms of Service</h1>
<div className="bg-card p-6 rounded-lg shadow-md">
<p className="text-muted-foreground">
This is a placeholder for the terms of service page. Our terms and
conditions will be detailed here soon.
</p>
<div className="max-w-4xl mx-auto">
<h1 className="text-3xl font-semibold mb-6">Terms of Service</h1>
<div className="bg-card p-6 rounded-lg shadow-md">
<p className="text-muted-foreground">
This is a placeholder for the terms of service page. Our terms and
conditions will be detailed here soon.
</p>
</div>

Copilot uses AI. Check for mistakes.
export default function PrivacyPage() {
return (
<div className="mx-auto max-w-7xl px-4 sm:px-6 lg:px-8 py-8">
<section className="w-full md:py-32 py-12 flex justify-center items-center min-h-screen">
Copy link

Copilot AI May 25, 2025

Choose a reason for hiding this comment

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

[nitpick] Similar to the Terms page, this section lacks a max-width wrapper. Adding a centered container (e.g., max-w-4xl mx-auto) would keep line lengths manageable.

Suggested change
<section className="w-full md:py-32 py-12 flex justify-center items-center min-h-screen">
<section className="w-full max-w-4xl mx-auto md:py-32 py-12 flex justify-center items-center min-h-screen">

Copilot uses AI. Check for mistakes.
Comment on lines 4 to 9
<h1 className="text-3xl font-semibold mb-6">Contribution Guidelines</h1>
<div className="bg-card p-6 rounded-lg shadow-md">
<p className="text-muted-foreground">
This is a placeholder for the contribution guidelines page. Detailed
instructions on how to contribute will be added soon.
</p>
Copy link

Copilot AI May 25, 2025

Choose a reason for hiding this comment

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

[nitpick] This page also dropped the original container width limit. Consider reinstating a max-width child container for consistent layout across pages.

Suggested change
<h1 className="text-3xl font-semibold mb-6">Contribution Guidelines</h1>
<div className="bg-card p-6 rounded-lg shadow-md">
<p className="text-muted-foreground">
This is a placeholder for the contribution guidelines page. Detailed
instructions on how to contribute will be added soon.
</p>
<div className="max-w-4xl w-full mx-auto">
<h1 className="text-3xl font-semibold mb-6">Contribution Guidelines</h1>
<div className="bg-card p-6 rounded-lg shadow-md">
<p className="text-muted-foreground">
This is a placeholder for the contribution guidelines page. Detailed
instructions on how to contribute will be added soon.
</p>
</div>

Copilot uses AI. Check for mistakes.
</p>
<section className="w-full md:py-32 py-12 flex flex-col items-center min-h-screen">
<div className="max-w-4xl w-full px-4">
<h1 className="text-4xl font-semibold mb-8">Documentation</h1>
Copy link

Copilot AI May 25, 2025

Choose a reason for hiding this comment

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

[nitpick] The heading size here (text-4xl) differs from other pages that use text-3xl. Aligning heading sizes across pages can improve visual consistency.

Suggested change
<h1 className="text-4xl font-semibold mb-8">Documentation</h1>
<h1 className="text-3xl font-semibold mb-8">Documentation</h1>

Copilot uses AI. Check for mistakes.
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.

2 participants