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

Landing page #15

Merged
merged 16 commits into from
Sep 16, 2024
Merged

Landing page #15

merged 16 commits into from
Sep 16, 2024

Conversation

tomaspalma
Copy link
Member

@tomaspalma tomaspalma commented Jul 20, 2024

Closes #1.

Preview: https://website-landing-page.up.railway.app/

During the meeting, there was a doubt about why the background in mobile was stretching so much. This was because all of the components inside of the <Home> component were being included in the <div> where we were putting the background in. Because, as for now, that background as the landing page is only to be present in the <Home> component, we chose to put a variable showNavbar in the <AppLayout> that defaults to true, so that in the <Home> component we can put that to false and render the <Navbar> inside of the <Home> component in order to be able to easily put the background only behind certain elements.

We checked the HTML validator and it is valid to have <nav> tags inside <main>. If you come up with a better approach, feel free to say so.

image

image

@limwa
Copy link
Member

limwa commented Jul 22, 2024

Hi, awesome work so far! Regarding the implementation, I've noticed that there are some inconsistencies with the mockups, but nothing too major.

  1. The navbar has a background when it should be transparent.
  2. The background on mobile is too zoomed in, but I'll try to see what the image department wants to do in this situation.
  3. The ENEI logo is too small on mobile, it needs to be a bit bigger.

@jose-carlos-sousa
Copy link
Contributor

Hi, awesome work so far! Regarding the implementation, I've noticed that there are some inconsistencies with the mockups, but nothing too major.

1. The navbar has a background when it should be transparent.

2. The background on mobile is too zoomed in, but I'll try to see what the image department wants to do in this situation.

3. The ENEI logo is too small on mobile, it needs to be a bit bigger.

I have a question about the navbar design. Currently, it is fixed to the top of the page. Making it transparent could lead to visibility issues with the content as users scroll. Should the navbar remain fixed at the top, or should it disappear when scrolling down?

@limwa
Copy link
Member

limwa commented Jul 24, 2024

Hey @jose-carlos-sousa. You don't need to worry about that for now and just leave the navbar transparent and not scrolling with the rest of the page. Either way, I'll ask the image department so that we know the answer when the time to implement the rest of the page comes.

@jose-carlos-sousa jose-carlos-sousa requested a review from a team July 27, 2024 11:04
@limwa limwa self-requested a review July 29, 2024 17:07
@limwa limwa force-pushed the feature/landing-page branch from 1a5613e to 4d50f65 Compare July 29, 2024 21:37
@limwa limwa force-pushed the feature/landing-page branch from 4d50f65 to c1da259 Compare July 29, 2024 22:32
@limwa
Copy link
Member

limwa commented Aug 11, 2024

Really awesome work 🎉

@limwa
Copy link
Member

limwa commented Aug 11, 2024

Screenshot_20240811-132253.png

Do you know what's that white bar on mobile? Appears when I scroll down

@tomaspalma
Copy link
Member Author

tomaspalma commented Aug 11, 2024

I believe the white bar is now fixed in mobile, do you think that the page being scrollable is an issue?

@limwa
Copy link
Member

limwa commented Aug 11, 2024

It's fine, don't worry! But if any of you is interested in going into webdev in the future, it might be an interesting challenge to solve.

Will check out the desktop version later today (mobile looks good) and then do a code review.

Copy link
Member

@limwa limwa left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you! 🎉

@limwa limwa requested a review from Naapperas August 12, 2024 17:11
@Naapperas
Copy link
Member

@limwa @tomaspalma I am not apporving this PR yet due to my comment, which was not necessarily a change request. Waiting on @ttoino also

Copy link
Member

@Naapperas Naapperas left a comment

Choose a reason for hiding this comment

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

LGTM

@limwa limwa merged commit 084c7f4 into main Sep 16, 2024
4 of 8 checks passed
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.

Implement Landing Page
5 participants