1

I want to make a page to upload the avatar. By default, I use the vector to show where the image will appear and then provide a button to upload the URL to change the avatar. That's all it is! But the script still doesn't work. Pleased to hear your feedback on how to fix it. Bless

<img id="put_image_here_bitch" src="https://cdn0.iconfinder.com/data/icons/finance-vol-2-4/48/77-512.png" alt="" width="100px" height="100px">
<div class="block" > The person who uploads this is cool
</div>

<button onclick="hideElement()">Click to upload photo by URL</button>

<div>
    <input id="input" autofocus class='hidden_element' style="display: none;" type="text" id="input">
</div>
<div>
    <button class='hidden_element' style="display: none;" onclick="uploadImage()">UPLOAD</button>

    
</div>

This is my script

 function hideElement(){
    var hide = document.getElementsByClassName('hidden_element');
    if (hide.style.display === "none") {
    hide.style.display = "block";
    } else {
    hide.style.display = "none";
    }
}

var uploadImage = function(){
    image = document.getElementById('input').value;
    showImage = document.getElementById('put_image_here_bitch').setAttribute('src', image);

    
};
Chris 410
  • 9
  • 3
  • 2
    `getElementsByClassName` returns a collection of elements, but you are treating it as if it is returning a single element. So yeah, it doesn't work. "Cannot read property 'display' of undefined." – Niet the Dark Absol May 13 '21 at 15:02

2 Answers2

1

As stated in the comments, .getElementsByClassName() returns a collection of elements, not a single element and your code attempts to call the style property of the collection, which doesn't exist.

Instead, you need to loop through the collection and operate on the elements within the collection individually, but don't use .getElementsByClassName() and instead use .querySelectorAll().

var hidden = document.querySelectorAll('.hidden_element');
function hideElement(){

  // Loop over the colleciton elements
  hidden.forEach(function(element){
    if (element.style.display === "none") {
    element.style.display = "block";
    } else {
    element.style.display = "none";
    }
  });
}

var uploadImage = function(){
    image = document.getElementById('input').value;
    showImage = document.getElementById('put_image_here_bitch').setAttribute('src', image);

    
};
<img id="put_image_here_bitch" src="https://cdn0.iconfinder.com/data/icons/finance-vol-2-4/48/77-512.png" alt="" width="100px" height="100px">
<div class="block" > The person who uploads this is cool
</div>

<button onclick="hideElement()">Click to upload photo by URL</button>

<div>
    <input id="input" autofocus class='hidden_element' style="display: none;" type="text" id="input">
</div>
<div>
    <button class='hidden_element' style="display: none;" onclick="uploadImage()">UPLOAD</button>

    
</div>

But, beyond that, you should also avoid using inline styles as they are the most specific way of setting a style and therefore the hardest to override. They also often require duplicated code to be written. Instead, use CSS classes as shown below:

// Get references to the DOM elements that you'll need to work with
const btnUpload = document.querySelector("button"); // find the first button
const hidden = document.querySelectorAll(".hidden");
const upload = document.querySelector(".upload");

// Do your event binding in JavaScript, not in HTML
btnUpload.addEventListener("click", hideElement);
upload.addEventListener("click", uploadImage);

function hideElement(){
  // Loop over the collection of hidden elements
  hidden.forEach(function(item){
    // See how much more simple it is to work with classes?
    item.classList.toggle("hidden");
  });
}

function uploadImage(){
    showImage = document.getElementById('put_image_here_bitch').setAttribute('src', input.value);    
};
.hidden { display:none; }
<img id="put_image_here_bitch" src="https://cdn0.iconfinder.com/data/icons/finance-vol-2-4/48/77-512.png" alt="" width="100px" height="100px">
<div class="block" > The person who uploads this is cool
</div>

<button>Click to upload photo by URL</button>

<div>
    <input id="input" autofocus class='hidden' type="text" id="input">
</div>
<div>
    <button class='hidden upload'>UPLOAD</button>   
</div>
Scott Marcus
  • 64,069
  • 6
  • 49
  • 71
  • what!? `getElementsByClassName()` is the second most efficient DOM reader .. are you kidding? – zergski May 13 '21 at 15:09
  • 1
    @zergski Because `.getElementsByClassName()` references a live node list, significant performance issues can occur because each time the variable that references that list is accessed, the DOM is searched again for matches. This is how the list is kept current. These use cases are not often and even when they are, a second call to `.querySelectorAll()` would be more controllable. – Scott Marcus May 13 '21 at 15:15
  • 1
    @zergski Did you even read the link I posted in my comment to your answer? – Scott Marcus May 13 '21 at 15:24
  • @ScottMarcus Thank you. Your comment not only solved my problem but also taught me to understand more of this language. Love – Chris 410 May 13 '21 at 15:38
  • 1
    @Chris410 You're welcome. Please up vote all helpful answers and consider marking this as "the" answer by clicking the check mark at the top left of the answer. – Scott Marcus May 13 '21 at 15:40
  • @scott I'm aware of live vs static collections. while in most use cases it's better to go with static, live collections are especially useful for dynamic pages as it's much less efficient to run a query each time you update the DOM. plus 'ClassName' is faster than its counterpart. it's just to say that you should never use something is very misleading =) – zergski May 13 '21 at 16:06
  • 1
    @zergski *...it's much less efficient to run a query each time you update the DOM* <-- But that's exactly what `.getElementsByClassName()` does and exactly why you should not use it. Instead, an explicit `.querySelectorAll()` call after a point where you know the DOM would have been updated would accomplish the task of working with dynamic DOM elements but eliminate the (always) unintended side effects that `.getElementsByClassNmae()` introduces. – Scott Marcus May 13 '21 at 16:12
  • @scott live NodeList objects can be created and returned faster by the browser because they don’t have to have all of the information up front while static NodeLists need to have all of their data from the start.. all I'm saying is that everything has its uses – zergski May 13 '21 at 16:41
  • 1
    @zergski That's a common misconception. This line `let stuff = document.getElementsByClassName("foo")` will return instantly because no DOM query is performed at this point. However, each and every time you then access the `stuff` variable, a DOM query is performed. So, while `.querySelectorAll()` does initially take longer, by the time you go to ***use*** the collection, it's already been populated. However, with `.getElementsByClassName()` there's no up front performance impact, but then you get hit ***every*** time you access the live collection, thus the issue and why you should avoid it. – Scott Marcus May 13 '21 at 16:47
  • I'm aware and completey understand what you're saying =) – zergski May 13 '21 at 16:54
  • 1
    @ScottMarcus I've tried and gone through it all but it seems the function only calls the first element of .hidden_element. Is there a way to solve this? – Chris 410 May 13 '21 at 17:23
  • 1
    @Chris410 My mistake. I misunderstood what you wanted. I've updated both of my examples to do it. – Scott Marcus May 13 '21 at 17:36
-1

simple.. getElementsByClassName() returns an HTMLCollection with all DOM elements containing that class. An HTMLCollection is like an array ( but not really ) containing element references.

thus you need to define which entry in the array you want to handle ( even if there's only one )

your code should work by simply adding [0] to your DOM read ( the '0' means the first element in the collection )

ex:

var hide = document.getElementsByClassName('hidden_element')[0];
zergski
  • 793
  • 3
  • 10