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

FirstLandingPageCommit #1

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

Conversation

mdua167
Copy link
Contributor

@mdua167 mdua167 commented Apr 6, 2024

No description provided.

@mdua167 mdua167 closed this Apr 6, 2024
@mdua167 mdua167 reopened this Apr 6, 2024
Comment on lines 8 to 19
const NavBar: React.FC = () =>
{
return(
<div style={{ position: 'fixed', top: 10, right: 0, padding: '10px' }}>
<ul style={{ listStyleType: 'none', margin: 0, padding: 10 }}>
<li style={{ display: 'inline', marginRight: '10px' }}><a href="/">Speakers</a></li>
<li style={{ display: 'inline', marginRight: '10px' }}><a href="/auth"> PuzleBang </a></li>
<li style={{ display: 'inline', marginRight: '10px' }}><a href="/contact">Login</a></li>
</ul>
</div>
);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's make this into its own component! So basically make a components folder, make a NavBar.tsx file in that folder, and then have that export a navbar component and import that here

Copy link
Contributor

Choose a reason for hiding this comment

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

also Manya, Aydan will probably mention this in a moment but let's use this component!

@AydanPirani AydanPirani self-requested a review April 6, 2024 17:55
Copy link
Contributor

@AydanPirani AydanPirani left a comment

Choose a reason for hiding this comment

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

hey manya, this is a super good start! leaving some general conventions/feedback here (didn't cover this fully at the meeting, so that's my bad!)

  1. since we're using chakra, we shouldn't be using html elements unless we absolutely have to (i.e. the tabs in the navbar can be a tab component, and the entire navbar could be a TabList! things like that would make it a lot easier to style and maintain in the future

  2. since we want to standardize things, we shouldn't have inline css within tsx files - it makes small changes really messy, since you need to go through and edit all instances of a single component. instead, we should define custom components wherever we need to style them, and you can import those normally (see how Button is being imported in this and this).

  3. since this is the first PR (WOOT WOOOT), we should add a grid-style layout to our pages! this involves creating a custom PageLayout component that takes in particular elements, and snaps them into positions. This way, we only need to worry about the "Main" content, as opposed to inserting the navbar at the top of every page.

also, i know this is a lot of text, so please please please take your time to go through it, and don't hesitate to ask questions if you need anything :))

Copy link

netlify bot commented Apr 13, 2024

👷 Deploy Preview for rp2024 processing.

Name Link
🔨 Latest commit 886cbca
🔍 Latest deploy log https://app.netlify.com/sites/rp2024/deploys/661ab960ad5fb100089899d3

Copy link

netlify bot commented Apr 13, 2024

Deploy Preview for rp2024 ready!

Name Link
🔨 Latest commit d4589ea
🔍 Latest deploy log https://app.netlify.com/sites/rp2024/deploys/6632d7cace1f870008e62636
😎 Deploy Preview https://deploy-preview-1--rp2024.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

<section className="slider">
{slides.map((slide, index) => {
return (
<div
Copy link
Contributor

Choose a reason for hiding this comment

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

box

key={index}
style={{ display: index === current ? 'block' : 'none' }}
>
<img
Copy link
Contributor

Choose a reason for hiding this comment

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

Image