0
\$\begingroup\$

So, I've a code that is below which works just fine:

<html>
  <head>
    <title> Javascript Practice </title>
  </head>
  <body>
    <div id="myDiv">
    </div>
    <script>
      var a = 0;
      var b = 255;
      var c = 0;
      var color2 = 'RGB(' + a + ',' + b + ',' + c + ')';

      document.getElementById("myDiv").style.backgroundColor = color2;
      document.getElementById("myDiv").style.width = 50;
      document.getElementById("myDiv").style.height = 300;
    </script>
  </body>
</html>

I've got a lot of feedback saying that when accessing DOM style property, I need to set value as strings such as setting DOM.style.width not to a number like I've done here but appending w + "px"

I'm puzzled because I'm using Brackets as a code editor and deploying this on chrome. My code runs just fine and every time I load the HTML page, it initializes a div of the required dimensions. Everyone tells me it won't work which is what's confusing me.

TIA.

\$\endgroup\$
4
  • 2
    \$\begingroup\$ Welcome to Code Review! We need to know what the code is intended to achieve. To help reviewers give you better answers, please add sufficient context to your question, including a title that summarises the purpose of the code. We want to know why much more than how. The more you tell us about what your code is for, the easier it will be for reviewers to help you. The title needs an edit to simply state the task, rather than your concerns about the code. \$\endgroup\$
    – Toby Speight
    Commented Mar 11, 2020 at 9:56
  • \$\begingroup\$ @Mr.Polywhirl no better way to demonstrate what results I'm getting. Look at the video that I've made - drive.google.com/open?id=1_p_-Tn0OlEpC_L_a2ov81VENzsbY_qWn \$\endgroup\$
    – Pradyut Vatsa
    Commented Mar 11, 2020 at 11:58
  • \$\begingroup\$ @TobySpeight Thanks for the feedback. Have tried to edit the title to reflect what I'm trying to achieve with the code. \$\endgroup\$
    – Pradyut Vatsa
    Commented Mar 11, 2020 at 12:02
  • \$\begingroup\$ Code Review requires concrete code from a project, with enough code and / or context for reviewers to understand how that code is used. Pseudocode, stub code, hypothetical code, obfuscated code, and generic best practices are outside the scope of this site. Please take a look at the help center. \$\endgroup\$
    – Mast ♦
    Commented Mar 11, 2020 at 13:53

1 Answer 1

Reset to default
0
\$\begingroup\$

Inline style lengths require units. In quirks mode, browsers will assume pixels as the unit, if provided with an integer instead of a length. If you are not sure how the browser will handle the value, specify the units.

The CSS parser interprets unitless numbers as px (except for line-height and any other properties where they have distinct meaning, and except in shorthands).

Bonus: Here is an extensible version of your script.

const state = {
  color : { r : 0, g : 255, b : 0 },
  dimensions : { width : 50, height : 300 }
}

const applyState = (el, state) => {
  Object.assign(el.style, {
    backgroundColor: `rgb(${state.color.r}, ${state.color.g}, ${state.color.b})`,
    width: `${state.dimensions.width}px`,
    height: `${state.dimensions.height}px`
  })
}

applyState(document.getElementById('my-div'), state)
<div id="my-div"></div>

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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