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

Main #521

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

Main #521

wants to merge 3 commits into from

Conversation

xingyin2024
Copy link

Week 2 | Project assignment

This is a news site about Architecture design and image which is built by Xing Yin and updated 20240825.

@JennieDalgren JennieDalgren self-assigned this Aug 27, 2024
@xingyin2024
Copy link
Author

Here is my website link
https://xingsnews.netlify.app/

Copy link
Collaborator

@JennieDalgren JennieDalgren left a comment

Choose a reason for hiding this comment

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

Great job with your News project. For next time, remember to add your Netlify link in the Readme file too. Here are some feedback:

Responsive Design:

You’ve incorporated media queries well to adjust the layout for mobile and tablet. One suggestion would be to use a min-width approach rather than max-width for media queries. This makes it easier to handle future breakpoints and device ranges.

Example:

@media (min-width: 768px) {
  .news-grid {
    grid-template-columns: repeat(2, 1fr);
  }
}

Redundant Code:

There are a few places where the CSS could be cleaned up. For instance, some properties are repeated or unused. Take the time to go through the styles and remove any redundancy to keep your CSS lean and efficient.

Example:

.header {
  font-size: 20px;
  color: black;
}
.header {
  margin-bottom: 20px;
}

These two declarations could be combined into one block.

In general, your work is off to a great start! Keep up the good work. 😊

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