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

Phoebe/research page banner #43

Merged
merged 7 commits into from
Jan 22, 2025
Merged

Phoebe/research page banner #43

merged 7 commits into from
Jan 22, 2025

Conversation

phoebezhou222
Copy link
Collaborator

@phoebezhou222 phoebezhou222 commented Jan 20, 2025

image

@phoebezhou222 phoebezhou222 self-assigned this Jan 20, 2025
@phoebezhou222 phoebezhou222 linked an issue Jan 20, 2025 that may be closed by this pull request
return (
<>
<div>
<img src="/research/rectangle8.png" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<img src="/research/rectangle8.png" />
<img className="w-screen aspect-[5/1]" src="/research/rectangle8.png" />

style the img so it matches the relative size on the figma

@minhhdtran
Copy link
Contributor

make sure to call the component in the correct page; currently, both the research components are being called in the home page. please move them to src/app/research/page.tsx

@minhhdtran
Copy link
Contributor

also, make sure to appropriately name images; you can just rename rectangle8 to researchbanner or something similar

Copy link
Contributor

Choose a reason for hiding this comment

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

rename this image

return (
<>
<div>
<img className="aspect-[5/1] w-screen" src="/research/rectangle8.png" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Use Image tag from next/image instead of HTML img tag

return (
<>
<div>
<img className="aspect-[5/1] w-screen" src="/research/rectangle8.png" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Import this image as well it should be something like

`import { ResearchBanner } from "@/public/research/researchbanner.webp"

@stanleylew5
Copy link
Contributor

Also merge whichever pr is associated with yours. It should convert your image to a webp file
image

<div>
<Image
src={ResearchConference}
className="aspect-[5/1] w-screen"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
className="aspect-[5/1] w-screen"
className="w-screen"

Comment on lines 7 to 8
<Research />
<ResearchImage />
Copy link
Contributor

Choose a reason for hiding this comment

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

Call research image above the research component

Copy link
Contributor

@stanleylew5 stanleylew5 left a comment

Choose a reason for hiding this comment

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

lgtm!

@stanleylew5 stanleylew5 merged commit 28d26ea into dev Jan 22, 2025
5 checks passed
@stanleylew5 stanleylew5 deleted the phoebe/research_page_banner branch January 22, 2025 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add research page banner
3 participants