1
\$\begingroup\$

This function receives the id of an element (frame) and calculates the correct height and width so it fits inside its parent element, while keeping its aspect ratio. Before I had a way to calculate the frame ratio dynamically, but I decided it was easier to simply hard code it, it calculates it through naturalHeight and naturalWidth if it's an image, and use 0.5625 for the rest which are iframes, and that ratio makes sense for all of those. It works, tried it in different sizing scenarios, what I'm not 100% sure is if the math involved to calculate the correct width and height in all cases is correct.

In all cases the element should fill as much as possible.

function fix_frame(frame_id)
{
    var id = `#${frame_id}`

    var frame = $(id)

    if(frame_id === "media_image_frame")
    {
        var frame_ratio = frame[0].naturalHeight / frame[0].naturalWidth
    }

    else
    {
        var frame_ratio = 0.5625
    }

    var parent = frame.parent()
    var parent_width = parent.width()
    var parent_height = parent.height()
    var parent_ratio = parent_height / parent_width

    var frame_width = frame.width()
    var frame_height = frame.height()

    if(parent_ratio === frame_ratio)
    {
        frame.width(parent_width)
        frame.height(parent_height)
    }

    else if(parent_ratio < frame_ratio)
    {
        frame.width(parent_height / frame_ratio)
        frame.height(parent_height)
    }

    else if(parent_ratio > frame_ratio)
    {
        frame.width(parent_width)
        frame.height(parent_width * frame_ratio)
    }
}
\$\endgroup\$
1
\$\begingroup\$

Although you posted your code it would help if you told us what you want help with?

This is possible to do using CSS. I've seen multiple solitions, here is the first one I found on Google: http://www.mademyday.de/css-height-equals-width-with-pure-css.html

\$\endgroup\$
  • \$\begingroup\$ Basically I want to check if my algorithm is fine. It's possible to do it with CSS using object-fit: contain, I think. The problem with this is that it causes some problems when I used it, so I'm making somewhat my own implementation of it, at least temporarily. \$\endgroup\$ – madprops Sep 1 '18 at 19:07
  • \$\begingroup\$ Did you look at the link I posted? It does not use object-fit most solutions rely on setting a margin or padding as a percentage. For example setting bottom-margin: 100% will set the bottom margin to 100% of the width \$\endgroup\$ – Marc Rohloff Sep 1 '18 at 23:13
  • \$\begingroup\$ I noticed that, but it looks pretty hacky. I try to avoid hacky stuff cause it will probably break later on. \$\endgroup\$ – madprops Sep 2 '18 at 8:38
1
\$\begingroup\$

Style

Some style points...

JavaScript style conventions.

  • Block delimiters on the same line eg if (foo) { and } else {
  • camelCase for variables not snake_case
  • Add spaces between if and ( eg if ( not if( same for for, else, while, do, switch, return, etc should all have a white space separating them.
  • Hoist var declarations to the top of the function

Code quality styles

  • Use ; at the end of every line, the exception is if you can list every instance where not including it will cause problems. Most that can do this will end up using ;
  • Use const for variables that do not change
  • Don't add single use variables unless it improves readability. eg You have something like var id = "foo" then var frame = $(id). Would be better as const frame = $("foo");
  • Don't add unneeded code. You have similar to if (a === b) { } else if (a < b) { } else if (a > b) { } the last if statement is redundant as the execution will only get to that point if a > b
  • Don't add variable you don't use. EG you declare frame_width and frame_height but don't use them.
  • Learn to use ternaries as they help reduce the overall source code size and let you use const in rather than var or let when setting values depending on statements.

Aim to reduce the over all code size as it will reduce the chance of bugs and increase the readability of the whole project source.

Rewrites

Rewriting you code to have identical functionality.

function fixFrame(frameId) {
    const frame = $(`#${frameId}`);
    const frameRatio = frameId === "media_image_frame" ? frame[0].naturalHeight / frame[0].naturalWidth : 0.5625;
    const parent = frame.parent()
    const parentHeight = parent.height();
    const parentWidth = parent.width();
    const parentRatio = parentHeight / parentWidth;
    frame.width(parentRatio < frameRatio ? parentHeight / frameRatio : parentWidth);
    frame.height(parentRatio > frameRatio ? parentWidth * frameRatio : parentHeight);
}

Or I would write it using the function context to reduce source code complexity via implied aberrations

function fixFrame(id) {
    const frame = $(`#${id}`);
    const fRatio = id === "media_image_frame" ? 
        frame[0].naturalHeight / frame[0].naturalWidth : 
        0.5625;
    const parent = frame.parent()
    const pH = parent.height(), pW = parent.width(), pRatio = pH / pW;
    frame.width(pRatio < fRatio ? pH / fRatio : pW);
    frame.height(pRatio > fRatio ? pW * fRatio : pH);
}

Using abbreviations as the context of the function provides the information to know what pH, pW and pRatio and fRatio mean.

The declaration of fRatio is too long so I made it multi line. Note I had the ? and : at the line ends.

\$\endgroup\$
  • \$\begingroup\$ Cool. I don't personally agree with some of the conventions you mention, but it's nice to see other's perspective. Also you reduced it quite a bit. Thanks \$\endgroup\$ – madprops Sep 2 '18 at 8:39

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.