0
\$\begingroup\$

I am a UI/UX designer. I am working on the HTML of a website, using Sass/scss+bootstrap 3 project. It's my first project in sass/scss.

Now my developer's feedback is: HTML code is not up to mark. need improvements. He will code in Angular. I would like to know, where I am wrong?

 @mixin common{
      float: left;
      width: 100%;
    }
    @mixin border-radius($radius) {
      border-radius: $radius;
      -webkit-border-radius: $radius;
      -moz-border-radius: $radius;
      -ms-border-radius: $radius;
      -o-border-radius: $radius;
    }
    @mixin full-bg{
        background-position:center center;
        background-size:cover;
    }
    a.be-buddy{
        background: rgba($brand-primary, 0.7);
        color: $brand-white;
        font-size: $font-size-h4;
        font-weight: $headings-font-weight;
        padding: ($padding-large-vertical * 2);
        line-height: $line-height-computed;
        float: left;
        position: relative;
        z-index: $zindex-be-buddy;
        text-decoration: none;
        width:100%;
        text-align:center;
      &:hover{
        background: rgba($brand-primary, 1);
        text-decoration:none;
        }
    }
    .navbar-wrapper {
        position: absolute;
        top: 0;
        width: 100%;
        z-index:$zindex-navbar;
    }
    .login a{
       float:right;
       background: rgba($gray-base, 0.5);
       color: $brand-white;
       font-size: $font-size-h4;
       padding:($padding-large-vertical * 2);
       font-weight: $headings-font-weight;
       text-decoration: none;
       line-height: $line-height-computed;
     &:hover{
       background: rgba($gray-base, 0.6);
       text-decoration: none;
       }
    }
    .cart-icon{
       width:65px;
       height:65px;
       padding:$padding-large-vertical;
       margin:1.5em auto;
       display:block;
       background:$gray-lighter;
       color: $brand-success;
       font-size: $font-size-h2;
       font-weight: $headings-font-weight;
       position:relative;
       text-shadow:none;
     &:before{
       width:150px;
       height:1px;
       position:absolute;
       left:-170px;
       top:32px;
       content: "";
       border-bottom: 1px solid $gray-lighter;
       }
     &:after{
       width:150px;
       height:1px;
       position:absolute;
       right:-170px;
       top:32px;
       content: "";
       border-bottom: 1px solid $gray-lighter;
       }

    }
    .brands-strip{
       @include common;
       padding: $padding-large-vertical;
       position:relative;
     &:before{
       content: " ";
       background: url("../scss/images/zigzag-top.png") repeat-x;
       height:8px;
       position:absolute;
       top:-7px;
       left:0;
       width:100%;
       }
     &:after{
       content: " ";
       background: url("../scss/images/zigzag-bottom.png") repeat-x;
       height:8px;
       position:absolute;
       bottom:-7px;
       left:0;
       width:100%;
       }
    }
    .box-width{
       width:14.2%;
       height:( $padding-large-vertical * 6.4);
       float:left;
      img{
       display:block;
       margin:auto;
       }
    }
.video{
   background-image: url("../scss/images/grass.jpg");
    background-attachment: fixed;
    @include full-bg;
    padding: ($padding-large-vertical * 3) (0);
    overflow:hidden;
}
.rounded-box{
    @include common;
    background: rgba($brand-white, 0.8);
    @include  border-radius($border-radius-base);
    min-height:92px;
    color: $text-color;
    margin-top: $margin25;
    padding: $padding-large-vertical;
 &:last-child{
    margin-bottom: $margin25;
    }
  p{
    font-size: $font-size-h4;
    color: $text-color;
    width:100%;
    display: table-cell;
    vertical-align: middle;
    height:72px;
    }
}
.video-bg{
    background:url("../scss/images/ipad.png") no-repeat;
    background-size:cover;
    position:relative;
    bottom:-30px;
    width:610px;
    height:409px;
 & iframe{
    margin-left: 161px;
    margin-top: 35px;
    width: 410px;
    height: 270px;
    }
}

And HTML CODE:

<div class="navbar-wrapper">
      <div class="container"><div class="row">

        <div class="col-xs-7 col-sm-4 col-md-3"><div class="row">
           <a class="be-buddy" href="#"> <i aria-hidden="true" class="fa fa-shopping-bag"></i> Buddy wordene</a>
        </div></div>
        <div class="col-xs-5 visible-xs"><div class="row"><div class="login text-center">
                  <a href="#"> <i aria-hidden="true" class="fa fa-user"></i> Inloggen </a>    
         </div></div></div>
        <div class="col-xs-12 col-sm-8 col-md-9"><div class="row">
        <nav class="navbar navbar-static-top">

            <div class="navbar-header">
            <a href="#page-top" class="navbar-brand visible-xs"><img src="scss/images/Logo-Inverted.png" class="img-responsive" alt="superbuddy-logo"></a>
              <button type="button" class="navbar-toggle collapsed" data-toggle="collapse" data-target="#navbar" aria-expanded="false" aria-controls="navbar">
                <span class="sr-only">Toggle navigation</span>
                <span class="icon-bar"></span>
                <span class="icon-bar"></span>
                <span class="icon-bar"></span>
              </button>

            </div>
            <div id="navbar" class="navbar-collapse collapse">
              <ul class="nav navbar-nav">
                <li class="active"><a href="#">Over Ons</a></li>
                <li><a href="#about">Vacatures</a></li>
                <li><a href="#contact">Contact</a></li>

              </ul>
              <div class="login hidden-xs" id="login">
                  <a href="#"> <i aria-hidden="true" class="fa fa-user"></i> Inloggen </a>    
              </div>
            </div>
        </nav>
        </div></div>
        <div class="col-xs-6 col-xs-offset-3 col-sm-3 col-sm-offset-4 col-md-2 col-md-offset-5 hidden-xs"><a href="#page-top" class="navbar-brand"><img src="scss/images/logo.png" class="img-responsive" alt="superbuddy-logo"></a></div>
      </div></div>
    </div>

<section class="video">
      <div class="container"><div class="row">
            <div class="col-xs-12 col-sm-8 col-sm-offset-2 col-md-5 col-md-offset-0">
                 <div class="rounded-box">
                      <div class="col-xs-3 col-sm-3 col-md-3"><div class="row"> <img src="scss/images/delivery-icon.png" class="img-responsive" alt="delivery-icon"> </div></div>
                      <div class="col-xs-9 col-sm-9 col-md-9"> <p>Eerste bestelling gratis thuisbezorgd, daarna vanaf €5,00</p> </div>
                 </div>
                 <div class="rounded-box">
                      <div class="col-xs-3 col-sm-3 col-md-3"><div class="row"> <img src="scss/images/timing-icon.png" class="img-responsive" alt="timing-icon"></div> </div>
                      <div class="col-xs-9 col-sm-9 col-md-9"> <p>We bezorgen in tijdvakken van 30 minuten, al binnen 1,5 uur</p> </div>
                 </div>
                 <div class="rounded-box">
                      <div class="col-xs-3 col-sm-3 col-md-3"><div class="row"> <img src="scss/images/price-icon.png" class="img-responsive" alt="price-icon"></div> </div>
                      <div class="col-xs-9 col-sm-9 col-md-9"> <p>Minimaal bestelbedrag €10,00</p> </div>
                 </div>
            </div>

            <div class="col-xs-12 col-sm-10 col-sm-offset-1 col-md-7 col-md-offset-0 col-lg-6 col-lg-offset-1">
                 <div class="video-bg">

<iframe id="video" src="https://www.youtube.com/embed/QL7hmM228UU?rel=0&showinfo=0" frameborder="0" allowfullscreen></iframe>
                 </div>
            </div>
      </div></div>
</section>
\$\endgroup\$
2
\$\begingroup\$

Just few things to highlight here

  • Misusing Twitter bootstrap: The grid structure in bootstrap allows up to 12 columns across the page.

<div class="col-xs-7 col-sm-4 col-md-3">

Here you have define 14 columns which is wrong. I will assume you don't know the four classes of Grid Layout in Bootstrap : xs (for phones), sm (for tablets), md (for desktops), lg (for larger desktops)

proper implementation

<div class="row">
  <div class="col-xs-4">></div>
  <div class="col-xs-4"></div>
  <div class="col-xs-4"></div>
</div>
  • Avoid Complicated Nesting : I noticed you've something like this
<div class="container">
  <div class="row">

    <!-- other content-->
    </div>
  <div>

IMHO, you don't have to define a container for each of your div because the class provides a responsive fixed width container for each of your nesting elements . All you have to do is define a general css to restrict the size of the outer div and all other elements contained will be restricted.

  • Too much content in your navbar header: This line
 <nav class="navbar navbar-static-top">
            <div class="navbar-header">
            <a href="#page-top" class="navbar-brand visible-xs"><img src="scss/images/Logo-Inverted.png" class="img-responsive" alt="superbuddy-logo"></a>
              <button type="button" class="navbar-toggle collapsed" data-toggle="collapse" data-target="#navbar" aria-expanded="false" aria-controls="navbar">
                <span class="sr-only">Toggle navigation</span>
                <span class="icon-bar"></span>
                <span class="icon-bar"></span>
                <span class="icon-bar"></span>
              </button>

            </div>
   </nav>

The w3schools documentation Navigation bars -Bootstrap for navbar has always kept the navbar header light weighted` e.g

WebSiteName

  • Using Sections and class=row in bootstrap : To be honest I feelyou get caught up when using the two. FYI you can achieve what your section is all about using <div class="row"></div> . I don;t see you nesting <article> so you should be fine using the <div class="row"></div>.
  • Alternative to iframe you could use <object> example. The excEmbed HTML5 Youtube Video

    <object width="640" height="360">
        <param name="movie" value="http://www.youtube.com/embed/yt-video-id?html5=1&amp;rel=0&amp;hl=en_US&amp;version=3"/
        <param name="allowFullScreen" value="true"/>
        <param name="allowscriptaccess" value="always"/>
        <embed width="640" height="360" src="https://www.youtube.com/embed/QL7hmM228UU?rel=0&showinfo=0" class="youtube-player" type="text/html" allowscriptaccess="always" allowfullscreen="true"/>
    </object>

I hope this helps. Cheers

\$\endgroup\$
  • \$\begingroup\$ These all are basic bootstrap coding structure. What I miss was just proper formatting + folder/file structure of sass code. \$\endgroup\$ – Sharanpreet Jul 28 '16 at 13:22
  • \$\begingroup\$ @Sharanpreet my answer was directed towards your bootstrap usage. Your code is alright but needs to follow the bootstrap standard of usage \$\endgroup\$ – Siobhan Jul 28 '16 at 14:41

Not the answer you're looking for? Browse other questions tagged or ask your own question.