-2
\$\begingroup\$

I'm applying for a UI/UX position at a company that makes software for landlords, apartments, property managers, etc. After the interview the hiring manager gave me a project to work on. These are the instructions:

As an exercise I would like for you to create an updated look and feel of this properties website. The only requirements are for you to keep their existing logo. But feel free to change up the layout structure along with typography, color scheme etc. If you can work on the landing / home page design and integrate this page using bootstrap to create the HTML / CSS and email me the jpg and your HTML / CSS files.

This is the jpeg she attached.

This what I have so far.

I'm about 90% done just wanted some general feedback from any designers out there. Also, I wanted to see if I could simplify my JavaScript.

    let arr = [
    "http://www.onemissionbay.com/wp-content/uploads/2016/01/gallery15.jpg",
    "http://www.onemissionbay.com/wp-content/uploads/2016/01/gallery1.jpg",
    "http://www.onemissionbay.com/wp-content/uploads/2016/01/gallery2.jpg",
    "http://www.onemissionbay.com/wp-content/uploads/2016/01/gallery6.jpg",
    "http://www.onemissionbay.com/wp-content/uploads/2016/01/gallery8.jpg",
    "http://www.onemissionbay.com/wp-content/uploads/2016/01/gallery10.jpg",
    "http://www.onemissionbay.com/wp-content/uploads/2016/01/gallery11.jpg",
    "http://www.onemissionbay.com/wp-content/uploads/2016/01/gallery13.jpg",
    "http://www.onemissionbay.com/wp-content/uploads/2016/01/gallery14.jpg"
];

(function loop(i) {
    $(".main").css("background-image", `url(${arr[i]})`);
    if (i < 9) {
        setTimeout(function() {
            loop(++i);
        }, 12000);
    } else {
        loop(0);
    }
})(0);

let opened = null;
let first = true;

function fn(className) {
    if (opened !== null && opened !== className) {
        document.getElementById(opened).style.width = "0";
        openNav(className);
        first = false;
    } else {
        first ? openNav(className) : closeNav(className);
        first = !first;
    }

    if (opened != null) {
        $(".mimgContainer").fadeOut(600);
    } else {
        $(".mimgContainer").fadeIn(600);
    }
}

function openNav(cn) {
    opened = cn;
    document.getElementById(cn).style.width = "100%";
}
function closeNav(cn) {
    opened = null;
    document.getElementById(cn).style.width = "0";
}

function menu() {
    $(".number, .mlogo").hide();
    $(".hamburger").addClass("col-xs-2");
}

let mfirst = true;
function menu() {
    if (opened !== null) {
        closeNav(opened);
        mfirst ? mopenNav() : mcloseNav();
        mfirst = !mfirst;
        console.log(mfirst);
    } else {
        mfirst ? mopenNav() : mcloseNav();
        mfirst = !mfirst;
        console.log(mfirst);
    }
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>

\$\endgroup\$
  • 2
    \$\begingroup\$ never do free work for a "job interview." since you did it anyway, do not under any circumstances give it to them unless they compensate you. you are getting scammed. \$\endgroup\$ – I wrestled a bear once. Aug 14 '17 at 14:07
2
\$\begingroup\$

For performance, there are a few optimizations that could improve it:

Preload images

If you know you are gonna need all the images, preload them.

Without this your slider is gonna appear blank and/or jumpy for the first time any image is shown.

let arr = [
    "http://www.onemissionbay.com/wp-content/uploads/2016/01/gallery15.jpg",
    /* ... */
    "http://www.onemissionbay.com/wp-content/uploads/2016/01/gallery14.jpg"
  ]
  .map(function(src) {
    let img = document.createElement("img");
    img.src = src;
    return src;
  });

This is one example of caching/ preloading, but many more can be made.

Don't create inaccessible code

Anonymous function is nice and it is good practice to put specific parts in closed scopes, but this pattern can limit you if you are not careful.

Imagine a future feature where the slider doesn't shift when the user is doing something. As it stands you cannot interact with your slider.

Make sure to design your code accessibly.

var looper = {
  'i': 0,
  'timeout': 1200,
  'timeoutHandle': 0,
  'frames': 9,
  'loop': function(i, force) {
    //"%"" Give modulus of number.
    //It saves an if statement
    this.i = (i === void 0 ? this.i : i) % this.frames;
    // Proof that it works
    console.log(this.i);
    //$(".main").css("background-image", `url(${arr[i]})`);
    this.timeoutHandle = setTimeout(function() {
      this.i++;
      this.loop();
    }.bind(this), this.timeout);
  }
};
//Start loop
looper.loop(0);
//Stop loop (pause)
clearTimeout(looper.timeoutHandle);
//Restart loop
looper.loop();

Be better than jQuery

jQuery is a very good library and if you are working cross-browser, or even cross platform, it can make life easier.

Odds are they have thought of some pitfall you'll stumble into and prepared accordingly to ensure your jQuery functions behaves as expected.

With this being said, don't simply rely on jQuery for everything. Especially if you don't need to support all the way back to the days of Windows XP, because odds are you can make a better solution using modern methods.

Modern technology, like requestAnimationFrame, that jQuery doesn't use to be on the safer side of compatability can greatly boost performance on your site.

Other than that, you'll probably never use jQuery to its fullest so make sure it is a conscious decision to say "I need enough of this for the download size to be worth it".

And then make sure to use it so as to have a uniform code pattern.

jQuery("#"+cn).style("width","100%") instead of document.getElementById(cn).style.width = "100%"

And more ...

You can always go beyond and above, but think about the points above as follows:

  1. Loading stuff is very heavy. Can i optimize ahead of time or save stuff for further use?
  2. When someone inevitably have to work on this in the future, is the code in such a state that it is easy to read and expand upon?
  3. Looking back a #1, while you could load a library for every single feature, stop to think "Is this something i can make (better) myself? Is a library worth the download and clutter of usable memory on the client-side?"

See this article for some extra performance tips.

Good luck

P.S.:

RoTaRu has a very good point. If your javaScript breaks, the client isn't "just" left without a slideshow, but with a completely unusable site.

You must either gain knowledge of any device and browser a client will access your site from and develop accordingly (This is where jQuery shines) or make a classic HTTP fallback (I.E. Make the site work without JavaScript).

\$\endgroup\$
2
\$\begingroup\$

What immediately negatively stands out to me is the pure JavaScript navigation.

The way you have implemented it completely breaks the usability: Forward und back no longer word, nor will page reload or bookmarking.

Furthermore neither will Google (or other search engines) be able to index the site properly, nor will disabled people (who rely non-standard browsers) be able to use the site.

You should build the site, so that it works without any JavaScript, and then add the JavaScript for effects.

\$\endgroup\$
1
\$\begingroup\$

A short review;

  • mcloseNav should be mCloseNav if you follow lowerCamelCase. It should probably even be closeNav since that m smells like Hungarian notation.

  • You define the function menu twice, that doesn't look good

  • Using a ternary as an if statement is generally frowned upon (first ? openNav(className) : closeNav(className);, just write an if statement)

  • Right here you could actually use a ternary

    if (opened != null) {
      $(".mimgContainer").fadeOut(600);
    } else {
      $(".mimgContainer").fadeIn(600);
    }
    

    could be

    const effectDuration = 600;
    const specialEffect = opened ? 'fadeOut' : 'fadeIn';
    $(".mimgContainer")[specialEffect](effectDuration);
    
  • I would write

    closeNav(opened);
    openNav(className);
    

    instead of

    document.getElementById(opened).style.width = "0";
    openNav(className);
    
  • fn, as names go is not so great, also I don't see anything reference this function (same goes for menu)

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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