Skip to content

NW6 | Orlando_Morales | HTML-CSS | Module_Project | Week1 #636

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

Open
wants to merge 26 commits into
base: master
Choose a base branch
from

Conversation

OrlandoMoralesKuan
Copy link

Volunteers: Are you marking this coursework? You can find a guide on how to mark this coursework in HOW_TO_MARK.md in the root of this repository

Your Details

  • Your Name: Orlando Morales
  • Your City: Manchester
  • Your Slack Name: Orlando Morales

Homework Details

  • Module: NW6 | HTML-CSS | Backlog Sprint1
  • Week: 1

Notes

  • What did you find easy?

  • What did you find hard?

  • What do you still not understand?

  • Any other notes?

@netlify
Copy link

netlify bot commented Sep 25, 2023

Deploy Preview for cyf-module-project-html-css ready!

Name Link
🔨 Latest commit bb95a97
🔍 Latest deploy log https://app.netlify.com/sites/cyf-module-project-html-css/deploys/652a9001ad1c9f00089352c8
😎 Deploy Preview https://deploy-preview-636--cyf-module-project-html-css.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

@bunday bunday left a comment

Choose a reason for hiding this comment

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

Good work, you just need to fix some positioning and element rearrangement and we can review again. Do let me know over the slack channel if you need any clarification

css/style.css Outdated
@@ -15,5 +49,167 @@ body {
*
* - When using Flexbox, remember the items you want to move around need to be inside a parent container set to 'display: flex'
*/
.header, .footer {
padding: 0rem 1rem 0rem 1rem;
max-width: 1200px;
Copy link

@bunday bunday Oct 12, 2023

Choose a reason for hiding this comment

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

setting a max width as 1200px limit your design to fit screen size less or equal to 1200px. This is how it renders on larger screen, which isnt what you are trying to achieve. Please look into this, remove the max-width anywhere you have it and work on the structuring again
Screenshot 2023-10-12 at 18 21 01

Copy link
Author

Choose a reason for hiding this comment

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

I did the change

</div>
</section>

<section class="Logos">
Copy link

Choose a reason for hiding this comment

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

This section needs to be adjusted in such a way that The Image and the text box exist together in a single div like below.

<div>
  <img src="/img/icon-devices.svg" alt="device" class="icon">
  <h4 class="iconText">Internet for all devices</h4>
</div>

Copy link
Author

Choose a reason for hiding this comment

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

I did the changes, put the images and the texts in the same div

<link rel="stylesheet" href="store.css">
<link rel="shortcut icon" type="image/x-icon" href="favicon.ico">
</head>
<body>
Copy link

Choose a reason for hiding this comment

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

Your store page design is decent. The positioning issue lingers here as well
Screenshot 2023-10-12 at 18 26 09

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