Hot answers tagged

5

The biggest critique about the Javascript I have is that collapsedInput could be declared with const because it doesn't get re-assigned. It is possible that the code in the event listener for the input event could be simplified using classList.toggle(). If there is only one element with class name collapsed then perhaps it would be better to use an id ...


4

I would make the following recommendations to clean it up for future maintainability and readability: Clean up and remove the anonymous functions. They aren't really needed here and just create extra clutter, decreasing readability. Remove the additional setTimeout call and just call to show when you're done loading. Use more meaningful names. It was ...


3

Are there any security concerns with the current approach? No. You never interact with a server or a database. There is no way you could have any security concerns. Focusing on the use of innerHTML here as I believe it could be potentially dangerous. Should I be creating elements instead of using text based HTML insertion? No it does not matter. ...


3

width="1068px" height="132px" The width and height attributes can’t contain "px". width="1068" height="132" The alt value for the logo should not contain "logo", just the company name. <i class="fab fa-facebook-f"></i> Don’t use the i element if you just want to display an icon via CSS. The i element has a meaning which isn’t appropriate for ...


3

Besides small points like the use of &nbsp; to indent, <code> unsemantically, everything looks well organized with a lot of effort in styling. And that leads me to the main point: It would be better to separate text content and styling. And that would mean an XML format with a custom XML-to-HTML XSL transformation. There are some XML formats ...


2

Overall the code looks fine. I just have a couple suggestions about the JavaScript and CSS. JS querySelectorAll vs getElementsByClassName Generally document.getElementsByClassName will be quicker than document.querySelectorAll (see this post for more information) and the former also returns a live HTMLCollection so it wouldn't need to be queried each time....


2

There's really not much you can improve. My follow up question would be where and how did you decide on your breakpoints? I suggest using a bit more standardized sizes; the ones I pasted below are directly from the Bootstrap 4 Template https://getbootstrap.com/docs/4.3/layout/overview/ You do not need all 4 sizes, but these are a bit more standard (even ...


2

It looks ok to me. What you could improve is the order of your CSS statements. For example This: header { /* background-color: var(--focus-alt); */ background: linear-gradient(45deg, #4394d6, #43c5d6); padding: 2rem 1rem 1rem 1rem; box-shadow: 5px 5px 5px rgba(0, 0, 0, 0.05); margin-bottom: 0.5rem; color: #fff; } Would look a ...


1

Define your variables at the top of your script outside of the setTimeout. That way, you only need to define every variable once. Also, you can make them const, since they won't change. The same goes for the run() function. Declare it outside of the setTimeout. You just want to call it inside the setTimeout. I hope this is helpful for you. const wrp = ...


1

Whenever possible put the styling in the CSS. Avoid doing it in the JavaScript like this. For example, you could add it to your .dice style. (Note: same goes for other parts of your code) diceDOM.style.display = 'block'; Use shorthand +=. (Note: same goes for other parts of your code) roundScore =roundScore + dice + diceTwo; 'array' is a bad name for a ...


1

Consistently formatting your code will help it look professional. There are some slight variations in style but many conventions are well-established such as always putting a space before {. Be particularly strict about your indentation (everything inside setInterval needs one more) and don't mix tabs and spaces (such as the align-self lines). It's not ...


Only top voted, non community-wiki answers of a minimum length are eligible