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

feat: improve home layout #1670

Merged
merged 8 commits into from
Nov 21, 2024

Conversation

carlosstenzel
Copy link
Contributor

Align content

by #1668

Copy link

netlify bot commented Oct 26, 2024

Deploy Preview for expressjscom-preview ready!

Name Link
🔨 Latest commit 117e9e4
🔍 Latest deploy log https://app.netlify.com/sites/expressjscom-preview/deploys/673e901c6c5a1f000816d643
😎 Deploy Preview https://deploy-preview-1670--expressjscom-preview.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.

Copy link
Member

@bjohansebas bjohansebas left a comment

Choose a reason for hiding this comment

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

Please fix the mobile responsiveness

@bjohansebas bjohansebas requested a review from a team October 26, 2024 13:23
@carlosstenzel
Copy link
Contributor Author

carlosstenzel commented Oct 28, 2024

Please fix the mobile responsiveness

@bjohansebas I made some responsiveness adjustments for mobile

@chrisdel101
Copy link
Contributor

chrisdel101 commented Oct 28, 2024

I think the original issue you put up is good idea, but this is what I get now at this width.
Screenshot 2024-10-27 at 6 24 37 PM
Obv the nav squish has nothing to do with your changes

Looking at is a bit more the page setup is pretty weird - this is from before our time.
In my opinion the main page content should have a wrapper where the max-width is set, so like home-conent, but without the nav content in there, announcments and intro should go inside wrapper. Then you can just margin:auto or flex box the content inside the wrapper.

Or however you want to do it.

@carlosstenzel
Copy link
Contributor Author

I think the original issue you put up is good idea, but this is what I get now at this width.
Obv the nav squish has nothing to do with your changes

I'm not looking at the menu right now at the request of the @bjohansebas

@chrisdel101 I will analyze the other screen sizes, tks

@bjohansebas
Copy link
Member

it could be fixed by adding padding to the elements you changed, and please remove the changes to the footer, as this was already done in another PR.

@bjohansebas
Copy link
Member

Hi @carlosstenzel, I’ve made some changes to fix some issues with certain screen measurements, and I’ve used grid for the boxes. This improves the responsive design.

Feel free to change it if you don’t think it’s the best approach.

@bjohansebas bjohansebas changed the title feat - align content feat: improve layout home Nov 20, 2024
@bjohansebas bjohansebas changed the title feat: improve layout home feat: improve home layout Nov 20, 2024
@carlosstenzel
Copy link
Contributor Author

@bjohansebas

Perfect, thank you.

css/style.css Show resolved Hide resolved
@bjohansebas bjohansebas merged commit 7b74c10 into expressjs:gh-pages Nov 21, 2024
8 checks passed
This was referenced Nov 21, 2024
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.

3 participants