Skip to content

NW6 | Bakhat Begum | HTML-CSS | HTML-CSS-Module-Project | Week 1 #644

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 18 commits into
base: master
Choose a base branch
from

Conversation

BakhatBegum
Copy link

@BakhatBegum BakhatBegum commented Sep 29, 2023

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: Bakhat Begum
  • Your City: Manchester

Homework Details

  • Module:HTML-CSS-Module-Project
  • Week: 3

Notes

  • What did you find easy?
    Just some parts of HTML
  • What did you find hard?
    Figure caption
  • What do you still not understand?
    I need to practice more in Grid.
  • Any other notes?
    No thanks

@netlify
Copy link

netlify bot commented Sep 29, 2023

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

Name Link
🔨 Latest commit 6f8672b
🔍 Latest deploy log https://app.netlify.com/sites/cyf-module-project-html-css/deploys/6587338bedbee50008d1c86d
😎 Deploy Preview https://deploy-preview-644--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

@musayuksel musayuksel left a comment

Choose a reason for hiding this comment

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

@BakhatBegum it's important & very good practice to fill out the PR (Pull Request) template completely.
I added some comments to your codebase, which I think could be helpful for your continued improvement. Great job! Keep up the good work!

p {
margin-top: 0;
margin-bottom: 0;
}

Choose a reason for hiding this comment

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

Let's take a look at the styles here. You'll notice that they are exactly the same. Instead of repeating yourself, you can optimise your code by selecting both the h1 and p elements and applying the desired styling together. This approach is called DRY code, which stands for "Don't Repeat Yourself." It helps you keep your code simple, clear, and efficient. By following this practice, you can avoid unnecessary repetition, make your code easier to maintain, and achieve a more streamlined solution.

h1 ,p {
  margin-top: 0;
  margin-bottom: 0;
}


.nav__link:hover {
color: orange;
}

Choose a reason for hiding this comment

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

It's great to see the implementation of the hover effect. This small addition enhances the user experience by providing a visual response. Well done!

css/style.css Outdated
/*Button*/

button {
padding: 1rem 1rem 1rem 1rem;

Choose a reason for hiding this comment

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

The concept of DRY code applies to the padding declaration as well. Instead of repeating "1rem" four times, you can simplify it by using the shorthand notation: padding: 1rem;. This approach achieves the same result and improves code readability.

css/style.css Outdated
Comment on lines 114 to 117
width: 93.9%;
height: 38rem;
background-color: var(--grey-light);
padding-left: 79px;

Choose a reason for hiding this comment

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

While the choice of numeric values like "93.9%", "38rem", and "79px" may seem unconventional, it's great to see your innovative approach to experimenting with different measurements. However, for code simplicity and clarity, I would suggest considering more standard and easily understandable values.

src="img/karma-logo.svg"
alt="it is used for website"
class="header__logo"
/>

Choose a reason for hiding this comment

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

While it is not mandatory, considering the user experience, it is worth noting that the majority of webpages include a link to the home page within their logos.
When developing your future applications, you may want to take this into consideration as it can enhance the overall user experience and improve navigation.

class="img__footer"
src="img/instagram-icon.svg"
alt="you can visit"
/>

Choose a reason for hiding this comment

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

Just like your navigation bar, it would be beneficial to consider adding links to the social icons.

@BakhatBegum
Copy link
Author

Hi Musayuksel, thank you very mcuh. It is really helpful. I worked on these points in week 2 HTML-CSS-Module project because we still working on this project.

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