3
\$\begingroup\$

I have a webapp that follows responsive design and has following layout structure. The left layout appears when the window's width > 768px and right layout shows otherwise.

Layout difference based on window's width

Based on preliminary research I think there are no modern pure CSS ways of doing it, so I implemented some jQuery code to solve it.

$(window).resize(function() {
    if (window.innerWidth > 768) {
        fixedHeight = window.innerHeight - $('.search__results').position().top;
        $('.search__results').addClass('overflow-y-scroll').height(fixedHeight);
    } else {
        $('.search__results').removeClass('overflow-y-scroll').height('auto');
    }
});

My CSS class is as follows:

.overflow-y-scroll {
    overflow-y: scroll;
}

I would like to leave my code up here for review and seek for DRY-er code, some spots I think may be up for improvement.

768px being fixed, should it be called from a global variable where it is the single point for setting the breakpoints? If so, I wonder where it should be placed so CSS & JavaScript can both use it?

Should I name $('.search__results') as a variable? Perhaps even make it a more generic helper function? If so, does any one have any tips on how to write it?

I'm also curious as to what is usually the best practice when creating such responsive layouts.

\$\endgroup\$
  • \$\begingroup\$ You know the @media css tags? w3schools.com/cssref/css3_pr_mediaquery.asp \$\endgroup\$ – Jordumus Apr 30 '15 at 12:18
  • 1
    \$\begingroup\$ @Jordumus Please don't link to w3schools, they are not considered to be a reputable resource (see w3fools.com). \$\endgroup\$ – cimmanon Apr 30 '15 at 12:29
  • \$\begingroup\$ @cimmanon Wasn't aware of that, sorry. Another link: developer.mozilla.org/en-US/docs/Web/Guide/CSS/Media_queries \$\endgroup\$ – Jordumus Apr 30 '15 at 12:31
  • 1
    \$\begingroup\$ Where is the markup that goes with this? I am confused as to how you know the terminology to use (responsive), but you haven't bothered to actually do any research into this (namely, the key component for responsive designs is missing: media queries). \$\endgroup\$ – cimmanon Apr 30 '15 at 12:32
  • \$\begingroup\$ @cimmanon w3fools.com has changed their page to acknowledge that w3schools has addressed the concerns previously mentioned on that page. The page no longer says anything specifically about w3schools being disreputable aside from a vague note that it "still has issues." \$\endgroup\$ – Thriggle Apr 30 '15 at 19:17
3
\$\begingroup\$

Your provided code is fairly small, so there's not much to review, but nevertheless.


Your comment on making 768 a constant is a valid idea,

const MAX_WINDOW_WIDTH = 768;

...

if (window.innerWidth > MAX_WINDOW_WIDTH) {

for magic numbers (unexplained numbers meaning unexplained things), it's best to declare as a const, but your version of JavaScript may not support this, so you can also use var.


I don't particularly understand why you have double underlines in '.search__results', and to answer your question: you could declare it as a variable if you like, but it really wouldn't matter either way.

I think 'overflow-y-scroll' is a bad name for a CSS class as it's, basically the content of the class.

where it is the single point for setting the breakpoints?

You can set breakpoints in the relevant debugger on your browser, usually labelled Developer Options, or something similar.


You don't really need to worry about your code violating DRY principles, the if-else loop cases aren't really similar enough for that. If you had something like twenty identical lines in there, maybe so.


Good questions, and clean code, good work!

\$\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.