Critique Tribute to Emmeline Pankhurst

Critique Tribute to Emmeline Pankhurst
0

#1

Hi there

Here’s my tribute - celebrating centenary of UK Votes for Women - aimed for a simple responsive design.

(Hope these are the correct links…)

Your feedback appreciated.

Oceanjanie


#2

Hi and congrats on completing the project!

The positive things that stand out to me are the slick CSS oval portrait and the use of colored text

I’m no pro but here’s a couple things I noticed you may want to address:

  • Lack of padding/margin at the top of the page, separating the jumbotron from the top of the page.
  • When the site is viewed fullscreen, the jumbotron takes up nearly 100% of the horizontal space. Maybe this is just on my screen (27") but there is a lot of negative grey space, that takes the eye away from the main content.
  • Your a link pointing back to your freeCodeCamp profile is broken. Looks like you are missing the closing bracket on your opening a tag

Also, consider taking some of your CSS out of the HTML section and figuring out how to assign it to your HTML by just placing it in the CSS section.

Good job! and good luck coding!


#3

Hi Cewert

Many thanks for these pointers. I am attending a local coding meet up tomorrow so will follow up on your points while I have their support closeby.

Jane


#4

That sounds like a great opportunity, Jane. Have fun and good luck!

Charlie


#5

Hi I got help on how to fix most of the issues. Thanks again.

Cheers

Jane


#6

You have a type/syntax error in the following html. You should have class= not class:
Also, you need to decide if you want a rounded image or a rectangle image. You can make the image rounded with bootstrap using the “rounded-circle” class not the “rounded” class. Then you could get rid of the border-radius: 50% in the style attribute. Also, since you are using Bootstrap 4, you do not need the img-responsive, because img-fluid is the newer version syntax.

<img class: "img-responsive img-fluid rounded mx-auto d-block" style="border-color: #585858; border-width: 6px; border-style: solid; border-radius: 50%" src="https://collectionimages.npg.org.uk/std/mw04857/Emmeline-Pankhurst.jpg" alt="Portrait of Emmeline Pankhurst: National Portrait Gallery">

Also, fullscreen everything seems to be centered. However, if the screen size shrinks a bit, you will notice the bottom section shifts to the left. It has to do with the offsets you are using below:

<div class="col-xs-12 col-sm-10 col-sm-offset-1 col-md-8 col-md-offset-2"> 

The correct syntax for what I believe you were trying to accomplish is:

<div class="col-xs-12 col-sm-10 offset-sm-1  col-md-8  offset-md-2"> 

Final suggestion is to not use the in-line style syntax. Put all your css in the CSS section and use classes if you need to override Bootstrap’s default values. It makes the HTML cleaner and easier to read and debug.