yichuandoris / MI-449-SS17-740-html-more-kinds-of-content-OSB7Pe

0 stars 0 forks source link

Project Feedback #1

Open yichuandoris opened 7 years ago

yichuandoris commented 7 years ago

@KatieMFritz Can you take a look at this? It's hosted here and meets the following criteria:

Hello, I am actually not sure if I get the requirement for this project correctly as my page looks...messy?! Could you please take a look at it and give me some guidance? Thanks. Also, I found the page seemed not be able to be found on the "hosted site," but there is not clue for me to track - I even didn't receive an email saying my page built is failed. Could you please help? Thanks.

KatieMFritz commented 7 years ago

Hi Doris!

I think your page looks fine - there's not much styling for this projects, so it's not supposed to look as nice as some of the other projects. You did a fantastic job with the new html elements! table can be tricky at first, but you nailed it. 🌟

Audio element wrapped with a

I noticed you wrapped your audio element inside an a element with a class of button. Was this just to give it a border? It does look nice, but it's best to avoid using elements just to change the way things look. Save a for when you want to make actual links (or in-page anchors, which I don't think we've discussed yet). If you want to add a nice border to an audio element in the future, there are ways to do that with CSS - your mentors will be happy to help if you need it. :smile:

Please remove the a element and let the audio do its own thing.

Consistent spacing and indentation

Your code looks very nice and easy to read, overall! There are a couple small inconsistencies to fix.

Linking CSS files

You did a great job linking to skeleton.css through the CDN link! 👍 I noticed you also have a Skeleton folder in your project, but you're not using it. It's best to delete any files or code you're not using in a project. Otherwise it can get confusing for Future You and other developers you're working with. In some cases, extra code or files might also slow down your website. Please delete any unnecessary files (including your skeleton CSS folder).

Using headings appropriately

Back on page 3 of the basic HTML elements lesson, we introduced headings. We didn't talk a whole lot about how to use them though. Here's some more information.

Headings (h1, h2, and so on) create a structured outline of the page. 📋 The name of the page should always be h1 (and usually, that should be the only h1 on the page). The main sections of the page should be labeled with h2, and sections that go under the h2 sections should be h3. Check out this example to see what I mean.

On your page, I would expect to see headings used like this:

Does that make sense? Please update all the headings on your page to create a logical outline.

Github pages bugs 🐛 🐞

Your hosted site is working fine for me today. Were you having the problem with the link here in Github, or in the MSU.lansing.codes app? We've noticed that Github pages has some bugs. These are some things to try if you're not seeing your site:

yichuandoris commented 7 years ago

Thank you @KatieMFritz !! I've made some revisions based on your comments, could you help to check it again? Also, the hosted site is working now:)

KatieMFritz commented 7 years ago

Almost! Everything looks good except the spacing issues from my previous comment. Clean those up and let me know. 😄

yichuandoris commented 7 years ago

Oh my... I thought I did it!! Now I've confirmed that they are gone. Thanks!

KatieMFritz commented 7 years ago

Yay! :shipit:

Not going to make you redo this, but what I meant in my first spacing comment was to change this:

<p>Green and speckled legs,<br>
  Hop on logs and lily pads<br>
  Splash in cool water.
</p>

to this:

<p>
  Green and speckled legs,<br>
  Hop on logs and lily pads<br>
  Splash in cool water.
</p>

See what I mean? 🐸